On 7/4/24 14:59, John Levon wrote:
> Pick up some more of the qemu_driver.c code so this function supports
> both CONFIG and LIVE updates.
>
> Note that qemuDomainUpdateDeviceFlags() passed vm->def to
> virDomainDeviceDefParse() for the VIR_DOMAIN_AFFECT_CONFIG case, which
> is technically incorrect; in the test driver code we'll fix this.
>
> Signed-off-by: John Levon <john.levon@xxxxxxxxxxx>
> ---
> src/test/test_driver.c | 54 +++++++++++++++++++++++++++++-------------
> 1 file changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 712bb20563..da682da9ad 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -10239,10 +10239,10 @@ testDomainAttachDevice(virDomainPtr domain, const char *xml)
>
>
> static int
> -testDomainUpdateDeviceConfig(virDomainDef *vmdef,
> - virDomainDeviceDef *dev,
> - unsigned int parse_flags,
> - virDomainXMLOption *xmlopt)
> +testDomainUpdateDevice(virDomainDef *vmdef,
> + virDomainDeviceDef *dev,
> + unsigned int parse_flags,
> + virDomainXMLOption *xmlopt)
> {
> virDomainDiskDef *newDisk;
> virDomainDeviceDef oldDev = { .type = dev->type };
> @@ -10316,12 +10316,16 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
> testDriver *driver = dom->conn->privateData;
> virDomainObj *vm = NULL;
> virObjectEvent *event = NULL;
> + virDomainDef *def = NULL;
> + virDomainDef *persistentDef = NULL;
> g_autoptr(virDomainDef) vmdef = NULL;
> - g_autoptr(virDomainDeviceDef) dev = NULL;
> + g_autoptr(virDomainDeviceDef) dev_live = NULL;
> + g_autoptr(virDomainDeviceDef) dev_config = NULL;
> int ret = -1;
> unsigned int parse_flags = 0;
>
> - virCheckFlags(VIR_DOMAIN_AFFECT_CONFIG, -1);
> + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> + VIR_DOMAIN_AFFECT_CONFIG, -1);
>
> if (!(vm = testDomObjFromDomain(dom)))
> goto cleanup;
> @@ -10337,9 +10341,24 @@ testDomainUpdateDeviceFlags(virDomainPtr dom,
> parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
> }
>
> - if (!(dev = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
> - NULL, parse_flags))) {
> - goto endjob;
> + /*
> + * NB: this has diverged from qemuDomainUpdateDeviceFlags(), which uses
> + * vm->def in both cases; technically the qemu driver should do the same.
> + */
I'll post a patch for this shortly. Let me remove it, it something that
belongs into commit message if anything.
> + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
> + goto cleanup;
> +
> + if (def) {
> + if (!(dev_live = virDomainDeviceDefParse(xml, def, driver->xmlopt,
> + NULL, parse_flags)))
> + goto endjob;
> + }
> +
> + if (persistentDef) {
> + if (!(dev_config = virDomainDeviceDefParse(xml, persistentDef,
> + driver->xmlopt, NULL,
> + parse_flags)))
> + goto endjob;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
Michal
