[libvirt] Regression in createQemuImg()@storage_backend_fs.c of 0.6.4

[Cole Robinson]

Ryota Ozaki wrote:
> Hi,
> 
> I've found a regression in using a backing store of a volume (qcow2)
> in a pool (dir). The following code of 0.6.4 hits my system that works
> with 0.6.3.
> 
> static int createQemuImg(virConnectPtr conn,
>                          virStorageVolDefPtr vol,
>                          virStorageVolDefPtr inputvol) {
> 
> (snip)
> 
>     const char *inputBackingPath = (inputvol ? inputvol->backingStore.path
>                                              : NULL);
> 
> (snip)
> 
>         /* XXX: Not strictly required: qemu-img has an option a different
>          * backing store, not really sure what use it serves though, and it
>          * may cause issues with lvm. Untested essentially.
>          */
>         if (!inputBackingPath ||
>             !STREQ(inputBackingPath, vol->backingStore.path)) {
>             virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
>                                   "%s", _("a different backing store can not "
>                                           "be specified."));
>             return -1;
>         }
> 
> 
> The inputBackingPath will be always NULL when it is called via
> virStorageBackendFileSystemVolBuild(). (The third argument is
> passed to that of createQemuImg as it is.)
> 
> static int
> virStorageBackendFileSystemVolBuild(virConnectPtr conn,
>                                     virStorageVolDefPtr vol) {
>     return _virStorageBackendFileSystemVolBuild(conn, vol, NULL);
> }
> 
> 
> I've addressed the regression with the patch
> 
> -        if (!inputBackingPath ||
> +        if (inputBackingPath &&
> 
> However, I'm not sure whether this fix satisfies the aim of
> the original code. Any idea?
> 

Ahh, that's my fault. The idea is that if cloning a volume, the backing
store from the original vol must match the backing store in the new XML.
Thanks for looking into this. I think the correct fix is:

diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index be6d011..3e26fce 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -1255,8 +1255,9 @@ static int createQemuImg(virConnectPtr conn,
          * backing store, not really sure what use it serves though, and it
          * may cause issues with lvm. Untested essentially.
          */
-        if (!inputBackingPath ||
-            !STREQ(inputBackingPath, vol->backingStore.path)) {
+        if (inputvol &&
+            (!inputBackingPath ||
+             !STREQ(inputBackingPath, vol->backingStore.path))) {
             virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                                   "%s", _("a different backing store
can not "
                                           "be specified."));


<minus the line wrapping>

- Cole