[PATCH] issue #312: Rename LVM while deleting logical storage pool
From: Aline Manera <alinefm@br.ibm.com> When creating a Logical storage pool, a LVM is created with the same name. And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name. Also convert an error message parameter to string instead of virStoragePool instance. Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import time + import libvirt from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()}) def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) + except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()}) -- 1.7.10.4
On 2014年03月05日 04:49, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
When creating a Logical storage pool, a LVM is created with the same name. And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name.
Also convert an error message parameter to string instead of virStoragePool instance.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import time + import libvirt
from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()})
def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) Well, I would say when this VG is renamed, user would felt confused and wondered if it is a different VG from previous one.
Also some referrence to the LV path will change: suppose I have a volume group called "abc" and has two LV "root" and "swap": [lvroyce@localhost ~]$ sudo vgdisplay --- Volume group --- VG Name abc System ID Format lvm2 [lvroyce@localhost ~]$ sudo lvdisplay --- Logical volume --- LV Path /dev/abc/swap LV Name swap VG Name abc --- Logical volume --- LV Path /dev/abc/root LV Name root VG Name abc After change the vgname from "abc" to "fedora", all its logical volumes path change: [lvroyce@localhost ~]$ sudo lvdisplay --- Logical volume --- LV Path /dev/fedora/swap --- Logical volume --- LV Path /dev/fedora/root LV Name root VG Name fedora This means all application depend on these volume path will be unsafe: e.g. a VM use an LV as its disk will not able to find its disk any more. Based on this, I suppose maybe when storage pool type is "logical" and not import an existent VG, can we validate the name is used by some VG and give out a clear indication?
+ except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()})
On 03/05/2014 04:13 AM, Royce Lv wrote:
On 2014年03月05日 04:49, Aline Manera wrote:
From: Aline Manera <alinefm@br.ibm.com>
When creating a Logical storage pool, a LVM is created with the same name. And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name.
Also convert an error message parameter to string instead of virStoragePool instance.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import time + import libvirt
from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()})
def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) Well, I would say when this VG is renamed, user would felt confused and wondered if it is a different VG from previous one.
Also some referrence to the LV path will change:
suppose I have a volume group called "abc" and has two LV "root" and "swap":
[lvroyce@localhost ~]$ sudo vgdisplay --- Volume group --- VG Name abc System ID Format lvm2
[lvroyce@localhost ~]$ sudo lvdisplay --- Logical volume --- LV Path /dev/abc/swap LV Name swap VG Name abc
--- Logical volume --- LV Path /dev/abc/root LV Name root VG Name abc
After change the vgname from "abc" to "fedora", all its logical volumes path change: [lvroyce@localhost ~]$ sudo lvdisplay --- Logical volume --- LV Path /dev/fedora/swap
--- Logical volume --- LV Path /dev/fedora/root LV Name root VG Name fedora
This means all application depend on these volume path will be unsafe: e.g. a VM use an LV as its disk will not able to find its disk any more.
Good point. Most of systems use UUID to mount devices but we can have problem on systems does not follow this principle.
Based on this, I suppose maybe when storage pool type is "logical" and not import an existent VG, can we validate the name is used by some VG and give out a clear indication?
ACK. I will do that instead of renaming the vg. Thanks.
+ except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()})
2014/3/5 4:49, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
When creating a Logical storage pool, a LVM is created with the same name.
And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. I am a bit confusing here. The volume group was created when the user
I assume you mean a volume group here. tried to create a storage pool from a raw device like "/dev/sda3". When the storage pool was undefined, it was reasonable to remove the volume group and make the raw device available again. Is it?
But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name. So the raw device will be used after the renaming and will not be available any more?
Also convert an error message parameter to string instead of virStoragePool instance.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import time + import libvirt
from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()})
def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) + except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()})
On 03/05/2014 04:50 AM, Shu Ming wrote:
2014/3/5 4:49, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
When creating a Logical storage pool, a LVM is created with the same name.
I assume you mean a volume group here.
Yes. I will update it
And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. I am a bit confusing here. The volume group was created when the user tried to create a storage pool from a raw device like "/dev/sda3". When the storage pool was undefined, it was reasonable to remove the volume group and make the raw device available again. Is it?
No. As I said deleting a storage pool does not affect the file system so the vg will keep there
But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name. So the raw device will be used after the renaming and will not be available any more?
Once a raw device is used for a logical pool it will not a raw device anymore - otherwise, the user manually do it on system But not through kimchi.
Also convert an error message parameter to string instead of virStoragePool instance.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import time + import libvirt
from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()})
def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) + except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()})
On 03/05/2014 04:50 AM, Shu Ming wrote:
2014/3/5 4:49, Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
When creating a Logical storage pool, a LVM is created with the same name.
I assume you mean a volume group here.
Yes. I will update it
And when deleting the storage pool the LVM keeps on the system. Which is the correct behavior as the storage pool is only a concept and deleting it should not affect the file system. I am a bit confusing here. The volume group was created when the user tried to create a storage pool from a raw device like "/dev/sda3". When the storage pool was undefined, it was reasonable to remove the volume group and make the raw device available again. Is it?
No. As I said deleting a storage pool does not affect the file system so the vg will keep there I would argue that. But creating a storage pool do affect the volume group system. So it is reasonable to undo the effect caused by creating a storage pool when deleting the storage pool. Also, only raw empty block devices can be used to create the storage pool in Kimchi, so it is safe to assume that
于 2014/3/6 1:53, Aline Manera 写道: the storage pool created by Kimchi can be deleted with the volume group under it released.
But it prevents the user to create a new logical pool with a name used before which is odd. So when deleting a logical pool, rename the LVM to a single name to allow user create a logical pool with any name. So the raw device will be used after the renaming and will not be available any more?
Once a raw device is used for a logical pool it will not a raw device anymore - otherwise, the user manually do it on system But not through kimchi.
Also convert an error message parameter to string instead of virStoragePool instance.
Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- src/kimchi/model/storagepools.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py index 76ab2d6..dd3c9a2 100644 --- a/src/kimchi/model/storagepools.py +++ b/src/kimchi/model/storagepools.py @@ -17,6 +17,8 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import time + import libvirt
from kimchi import xmlutils @@ -172,7 +174,8 @@ class StoragePoolModel(object): return 0 except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0008E", - {'name': pool, 'err': e.get_error_message()}) + {'name': pool.name(), + 'err': e.get_error_message()})
def _get_storage_source(self, pool_type, pool_xml): source = {} @@ -348,7 +351,17 @@ class StoragePoolModel(object): if pool.isActive(): raise InvalidOperation("KCHPOOL0005E", {'name': name}) try: + xml = pool.XMLDesc(0) + pool_type = xmlutils.xpath_get_text(xml, "/pool/@type")[0] pool.undefine() + # The user may want to create a logical pool with the same name + # used before but a LVM will already exist with this name + # So rename the former LVM to avoid future conflicts + if pool_type == 'logical': + lvm_name = "%s-%s" % (name, str(int(time.time()*1000))) + vgrename_cmd = ["vgrename", name, lvm_name] + run_command(vgrename_cmd) + except libvirt.libvirtError as e: raise OperationFailed("KCHPOOL0011E", {'name': name, 'err': e.get_error_message()})
participants (3)
-
Aline Manera -
Royce Lv -
Shu Ming