On Fri, Feb 17, 2017 at 15:49:14 +0100, Pavel Hrdina wrote:
> If virDomainDelIOThread API was called with VIR_DOMAIN_AFFECT_LIVE
> and VIR_DOMAIN_AFFECT_CONFIG and both XML were already a different
> it could result in removing iothread from config XML even if there
> was a disk using that iothread.
> 
> Signed-off-by: Pavel Hrdina <[email protected]>
> ---
>  src/qemu/qemu_driver.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 189c10ead5..1a7cc12874 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5740,6 +5740,25 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
>  }
>  
>  static int
> +qemuDomainDelIOThreadCheck(virDomainDefPtr def,
> +                           unsigned int iothread_id)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        if (def->disks[i]->iothread == iothread_id) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("cannot remove IOThread %u since it "
> +                             "is being used by disk '%s'"),
> +                           iothread_id, def->disks[i]->dst);
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int
>  qemuDomainChgIOThread(virQEMUDriverPtr driver,
>                        virDomainObjPtr vm,
>                        unsigned int iothread_id,
> @@ -5773,6 +5792,9 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
>              if (qemuDomainHotplugAddIOThread(driver, vm, iothread_id) < 0)
>                  goto endjob;
>          } else {
> +            if (qemuDomainDelIOThreadCheck(def, iothread_id) < 0)
> +                goto endjob;
> +
>              if (qemuDomainHotplugDelIOThread(driver, vm, iothread_id) < 0)
>                  goto endjob;
>          }
> @@ -5797,6 +5819,9 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
>                  goto endjob;
>              }
>  
> +            if (qemuDomainDelIOThreadCheck(persistentDef, iothread_id) < 0)
> +                goto endjob;

This check should be done prior to modifying the live state so that you
don't modify it and the API then returns failure.

On the other hand ... It's not worse than it was. Fixing the corner case
would require also moving the call to virDomainIOThreadIDFind.

Consider this as an ACK

Peter

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to