Re: [PATCH] virsh: Check wether found volume is member of the specified storage pool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 30, 2014 at 03:39:36PM +0200, Peter Krempa wrote:
When looking up storage volumes virsh uses multiple lookup steps. Some
of the steps don't require a pool name specified. This resulted into a
possibility that a volume would be part of a different pool than the
user specified:

Let's have a /var/lib/libvirt/images/test.qcow image in the 'default'
pool and a second pool 'emptypool':

Currently we'd return:
 $ virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
 Name:           test.qcow
 Type:           file
 Capacity:       100.00 MiB
 Allocation:     212.00 KiB


I believed that the --pool parameter for vol-info (and *some* others)
was only a hint in case you had more volumes with the same name
(specifying absolute path with a pool doesn't make any sense).  That
would mean the BZ is notabug actually, but let's assume such users
exist...

After the fix:
$ tools/virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
error: Requested volume '/var/lib/libvirt/images/test.qcow' found in a different pool (default) than specified


I'd say this is rather noisy.  How about changing it to ...'%s' not
found in pool '%s'...  or "is not in pool '%s'?

ACK after release with or without the change mentioned above.

Martin

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1088667
---
tools/virsh-volume.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 55bf6f0..6416ba6 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -104,6 +104,26 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
                            "might help"), n, portamento);
    }

+    /* If the pool was specified, then make sure that the returned
+     * volume is from the given pool */
+    if (pool && vol) {
+        virStoragePoolPtr volpool = NULL;
+
+        if ((volpool = virStoragePoolLookupByVolume(vol))) {
+            if (STRNEQ(virStoragePoolGetName(volpool),
+                       virStoragePoolGetName(pool))) {
+                vshResetLibvirtError();
+                vshError(ctl,
+                         _("Requested volume '%s' found in a different "
+                           "pool (%s) than specified"),
+                         n, virStoragePoolGetName(volpool));
+                virStorageVolFree(vol);
+                vol = NULL;
+            }
+            virStoragePoolFree(volpool);
+        }
+    }
+
    if (pool)
        virStoragePoolFree(pool);

--
1.9.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]