[libvirt] [PATCH 0/3] qemu: support update graphic device persistently

We can change vnc password by using virDomainUpdateDeviceFlags API with live flag. But it can't be changed with config flag. 1/3: improve the error number when update graphics failed 2/3: refactor the function qemuDomainFindGraphics for the future patch 3/3: support updating vnc/spice auth arguments persistently Wang Rui (3): qemu: report properer error number when change graphics failed qemu: revise qemuDomainFindGraphics to be reused in the future patch qemu: make persistent update of graphics device supported src/conf/domain_conf.c | 3 ++- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.c | 23 +++++++++++------------ src/qemu/qemu_hotplug.h | 3 +++ 4 files changed, 53 insertions(+), 14 deletions(-) -- 1.7.12.4

Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/qemu/qemu_hotplug.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b9a0cee..1c75861 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2347,7 +2347,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, switch ((virDomainGraphicsListenType) newlisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: if (STRNEQ_NULLABLE(newlisten->address, oldlisten->address)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ? _("cannot change listen address setting on vnc graphics") : _("cannot change listen address setting on spice graphics")); @@ -2377,12 +2377,12 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) || (!dev->data.vnc.autoport && (olddev->data.vnc.port != dev->data.vnc.port))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change port settings on vnc graphics")); goto cleanup; } if (STRNEQ_NULLABLE(olddev->data.vnc.keymap, dev->data.vnc.keymap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change keymap setting on vnc graphics")); goto cleanup; } @@ -2423,13 +2423,13 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, (olddev->data.spice.port != dev->data.spice.port)) || (!dev->data.spice.autoport && (olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change port settings on spice graphics")); goto cleanup; } if (STRNEQ_NULLABLE(olddev->data.spice.keymap, dev->data.spice.keymap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change keymap setting on spice graphics")); goto cleanup; } -- 1.7.12.4

On 11/19/2014 05:50 AM, Wang Rui wrote:
Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/qemu/qemu_hotplug.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b9a0cee..1c75861 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2347,7 +2347,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, switch ((virDomainGraphicsListenType) newlisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: if (STRNEQ_NULLABLE(newlisten->address, oldlisten->address)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s",
But the arguments are valid, we just can't change them. I think 'operation unsupported' would be a better error code. Jan
dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ? _("cannot change listen address setting on vnc graphics") : _("cannot change listen address setting on spice graphics")); @@ -2377,12 +2377,12 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, if ((olddev->data.vnc.autoport != dev->data.vnc.autoport) || (!dev->data.vnc.autoport && (olddev->data.vnc.port != dev->data.vnc.port))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change port settings on vnc graphics")); goto cleanup; } if (STRNEQ_NULLABLE(olddev->data.vnc.keymap, dev->data.vnc.keymap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change keymap setting on vnc graphics")); goto cleanup; } @@ -2423,13 +2423,13 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, (olddev->data.spice.port != dev->data.spice.port)) || (!dev->data.spice.autoport && (olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change port settings on spice graphics")); goto cleanup; } if (STRNEQ_NULLABLE(olddev->data.spice.keymap, dev->data.spice.keymap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot change keymap setting on spice graphics")); goto cleanup; }

We want to use qemuDomainFindGraphics in the qemuDomainUpdateDeviceConfig in a future patch. So adjust its parameter. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/qemu/qemu_hotplug.c | 13 ++++++------- src/qemu/qemu_hotplug.h | 3 +++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1c75861..d39ed54 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2297,15 +2297,14 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, } - -static virDomainGraphicsDefPtr qemuDomainFindGraphics(virDomainObjPtr vm, - virDomainGraphicsDefPtr dev) +virDomainGraphicsDefPtr +qemuDomainFindGraphics(virDomainDefPtr def, virDomainGraphicsDefPtr dev) { size_t i; - for (i = 0; i < vm->def->ngraphics; i++) { - if (vm->def->graphics[i]->type == dev->type) - return vm->def->graphics[i]; + for (i = 0; i < def->ngraphics; i++) { + if (def->graphics[i]->type == dev->type) + return def->graphics[i]; } return NULL; @@ -2317,7 +2316,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainGraphicsDefPtr dev) { - virDomainGraphicsDefPtr olddev = qemuDomainFindGraphics(vm, dev); + virDomainGraphicsDefPtr olddev = qemuDomainFindGraphics(vm->def, dev); int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 1c9ca8f..176fde0 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -55,6 +55,9 @@ int qemuDomainAttachHostDevice(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev); +virDomainGraphicsDefPtr +qemuDomainFindGraphics(virDomainDefPtr def, + virDomainGraphicsDefPtr dev); int qemuDomainChangeGraphics(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainGraphicsDefPtr dev); -- 1.7.12.4

We can change vnc password by using virDomainUpdateDeviceFlags API with live flag. But it can't be changed with config flag. Error is reported as below. error: Operation not supported: persistent update of device 'graphics' is not supported This patch supports the vnc/spice auth arguments changed with config flag. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/conf/domain_conf.c | 3 ++- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5f4b9f6..5879f54 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20847,7 +20847,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainControllerDefFormat(&buf, src->data.controller, flags); break; case VIR_DOMAIN_DEVICE_GRAPHICS: - rc = virDomainGraphicsDefFormat(&buf, src->data.graphics, flags); + rc = virDomainGraphicsDefFormat(&buf, src->data.graphics, + flags | VIR_DOMAIN_XML_SECURE); break; case VIR_DOMAIN_DEVICE_HUB: rc = virDomainHubDefFormat(&buf, src->data.hub, flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a84fd47..3096ae4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7397,11 +7397,48 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + virDomainGraphicsDefPtr graphics, newGraphics; virDomainDiskDefPtr orig, disk; virDomainNetDefPtr net; int pos; switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_GRAPHICS: + newGraphics = dev->data.graphics; + graphics = qemuDomainFindGraphics(vmdef, newGraphics); + if (!graphics) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find existing graphics type '%s' device to modify"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + VIR_FREE(graphics->data.vnc.auth.passwd); + graphics->data.vnc.auth.passwd = newGraphics->data.vnc.auth.passwd; + newGraphics->data.vnc.auth.passwd = NULL; + graphics->data.vnc.auth.validTo = newGraphics->data.vnc.auth.validTo; + graphics->data.vnc.auth.expires = newGraphics->data.vnc.auth.expires; + graphics->data.vnc.auth.connected = newGraphics->data.vnc.auth.connected; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + VIR_FREE(graphics->data.spice.auth.passwd); + graphics->data.spice.auth.passwd = newGraphics->data.spice.auth.passwd; + newGraphics->data.spice.auth.passwd = NULL; + graphics->data.spice.auth.validTo = newGraphics->data.spice.auth.validTo; + graphics->data.spice.auth.expires = newGraphics->data.spice.auth.expires; + graphics->data.spice.auth.connected = newGraphics->data.spice.auth.connected; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to change config on '%s' graphics type"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + break; + case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; pos = virDomainDiskIndexByName(vmdef, disk->dst, false); @@ -7455,7 +7492,6 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: -- 1.7.12.4

On 11/19/2014 05:50 AM, Wang Rui wrote:
We can change vnc password by using virDomainUpdateDeviceFlags API with live flag. But it can't be changed with config flag. Error is reported as below.
error: Operation not supported: persistent update of device 'graphics' is not supported
This patch supports the vnc/spice auth arguments changed with config flag.
Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/conf/domain_conf.c | 3 ++- src/qemu/qemu_driver.c | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5f4b9f6..5879f54 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20847,7 +20847,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainControllerDefFormat(&buf, src->data.controller, flags); break; case VIR_DOMAIN_DEVICE_GRAPHICS: - rc = virDomainGraphicsDefFormat(&buf, src->data.graphics, flags); + rc = virDomainGraphicsDefFormat(&buf, src->data.graphics, + flags | VIR_DOMAIN_XML_SECURE);
I'd rather add the flag to the 'flags' variable initialization. (It has no effect on formatting other devices anyway).
break; case VIR_DOMAIN_DEVICE_HUB: rc = virDomainHubDefFormat(&buf, src->data.hub, flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a84fd47..3096ae4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7397,11 +7397,48 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + virDomainGraphicsDefPtr graphics, newGraphics; virDomainDiskDefPtr orig, disk; virDomainNetDefPtr net; int pos;
switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_GRAPHICS: + newGraphics = dev->data.graphics; + graphics = qemuDomainFindGraphics(vmdef, newGraphics); + if (!graphics) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find existing graphics type '%s' device to modify"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + VIR_FREE(graphics->data.vnc.auth.passwd); + graphics->data.vnc.auth.passwd = newGraphics->data.vnc.auth.passwd; + newGraphics->data.vnc.auth.passwd = NULL; + graphics->data.vnc.auth.validTo = newGraphics->data.vnc.auth.validTo; + graphics->data.vnc.auth.expires = newGraphics->data.vnc.auth.expires; + graphics->data.vnc.auth.connected = newGraphics->data.vnc.auth.connected; + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + VIR_FREE(graphics->data.spice.auth.passwd); + graphics->data.spice.auth.passwd = newGraphics->data.spice.auth.passwd; + newGraphics->data.spice.auth.passwd = NULL; + graphics->data.spice.auth.validTo = newGraphics->data.spice.auth.validTo; + graphics->data.spice.auth.expires = newGraphics->data.spice.auth.expires; + graphics->data.spice.auth.connected = newGraphics->data.spice.auth.connected; + break; +
We can just free the old device and replace it with the new one, like we do for DEVICE_NET. (This will also make it work for other graphics types) Jan
+ default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to change config on '%s' graphics type"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + break; + case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; pos = virDomainDiskIndexByName(vmdef, disk->dst, false); @@ -7455,7 +7492,6 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON:
participants (2)
-
Ján Tomko
-
Wang Rui