[libvirt] [PATCH 10/13] qemu_driver: always check whether iothread is used by disk or not

[Peter Krempa]

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 <phrdina at redhat.com>
> ---
>  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170220/41f30c8a/attachment-0001.sig>