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

Re: [libvirt] [PATCH] storage: replace the deprecated option of qemu-img.



On Wed, Mar 09, 2011 at 08:27:41PM +0800, Osier Yang wrote:
> qemu-img silently disable "-e", so we can't use it for volume
> encryption anymore, change it into "-o encryption=on".
> 
> I'm afraid of it will inroduce compatibility problem for older
> qemu without "-o" option, but "-o" option is already used in the
> codes, seems it's fine.

The current code *only* uses '-o' after it has checked that
qemu-img actually supports it.

> * src/storage/storage_backend.c
> ---
>  src/storage/storage_backend.c |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 2eede74..c381444 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -778,7 +778,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>              imgargv[8] = vol->target.path;
>              imgargv[9] = size;
>              if (vol->target.encryption != NULL)
> -                imgargv[10] = "-e";
> +                imgargv[10] = "-o encryption=on";
>              break;
> 
>          case QEMU_IMG_BACKING_FORMAT_OPTIONS:
> @@ -786,13 +786,18 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>                  virReportOOMError();
>                  goto cleanup;
>              }
> +
> +            if (vol->target.encryption != NULL) {
> +                if (virAsprintf(&optflag, ",encryption=on") < 0) {
> +                    virReportOOMError();
> +                    goto cleanup;
> +                }
> +            }
> +
>              imgargv[6] = "-o";
>              imgargv[7] = optflag;
>              imgargv[8] = vol->target.path;
>              imgargv[9] = size;
> -            if (vol->target.encryption != NULL)
> -                imgargv[10] = "-e";
> -            break;
> 
>          default:
>              VIR_INFO("Unable to set backing store format for %s with %s",
> @@ -800,7 +805,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>              imgargv[6] = vol->target.path;
>              imgargv[7] = size;
>              if (vol->target.encryption != NULL)
> -                imgargv[8] = "-e";
> +                imgargv[8] = "-o encryption=on";
>          }
> 
>          ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
> @@ -817,7 +822,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>              NULL
>          };
>          if (vol->target.encryption != NULL)
> -            imgargv[6] = "-e";
> +            imgargv[6] = "-o encryption=on";
> 
>          ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
>      }

NACK, we must keep backwards compatibility and only use '-o' after we
have determined it is supported

It also looks like the first part of the method which calls 'qemu-img convert'
is broken because it is ignoring encryption entirely

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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