On 03/05/2014 04:13 AM, Royce Lv wrote:
On 2014年03月05日 04:49, Aline Manera wrote:
> From: Aline Manera <alinefm(a)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(a)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()})