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

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. * 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); } -- 1.7.4

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 :|

于 2011年03月09日 20:31, Daniel P. Berrange 写道:
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.
Oh, yes
* 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
Okay, will update. Thanks.
It also looks like the first part of the method which calls 'qemu-img convert' is broken because it is ignoring encryption entirely
Will fix it in Incidentally. Regards Osier

On Wed, Mar 09, 2011 at 20:27:41 +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.
* 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";
Are you sure qemu-img is able to parse this option and you don't need to add two args "-o" and "encryption=on" instead of passing it as a single argument? Jirka

于 2011年03月09日 20:50, Jiri Denemark 写道:
On Wed, Mar 09, 2011 at 20:27:41 +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.
* 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";
Are you sure qemu-img is able to parse this option and you don't need to add two args "-o" and "encryption=on" instead of passing it as a single argument?
Changed it, thanks, :-) Regards Osier
participants (3)
-
Daniel P. Berrange
-
Jiri Denemark
-
Osier Yang