Re: [PATCH v2] qemu: cold-unplug of sound

[Jidong Xia <xiajidong@xxxxxxxxxxxxxxxxxxxx>]

On 12/8/19 4:43 PM, Cole Robinson wrote:
> On 11/29/19 4:40 AM, Jidong Xia wrote:
> > With this patch users can cold unplug some sound devices.
> > use "virsh detach-device vm sound.xml --config" command.
> >
> > Signed-off-by: Jidong Xia <xiajidong@xxxxxxxxxxxxxxxxxxxx>
> > ---
> >   src/conf/domain_conf.c   | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   src/conf/domain_conf.h   |  3 +++
> >   src/libvirt_private.syms |  2 ++
> >   src/qemu/qemu_driver.c   |  9 ++++++++-
> >   4 files changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 9580884..4b28502 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2753,6 +2753,14 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
> >       VIR_FREE(def);
> >   }
> >
> > +virDomainSoundDefPtr
> > +virDomainSoundDefRemove(virDomainDefPtr def, size_t idx)
> > +{
> > +    virDomainSoundDefPtr ret = def->sounds[idx];
> > +    VIR_DELETE_ELEMENT(def->sounds, idx, def->nsounds);
> > +    return ret;
> > +}
> > +
> >   void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def)
> >   {
> >       if (!def)
> > @@ -14554,6 +14562,46 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt,
> >   }
> >
> >
> > +static bool
> > +virDomainSoundDefEquals(const virDomainSoundDef *a,
> > +                        const virDomainSoundDef *b)
> > +{
> > +    size_t i;
> > +
> > +    if (a->model != b->model)
> > +        return false;
> > +
> > +    if (a->ncodecs != b->ncodecs)
> > +        return false;
> > +
> > +    for (i = 0; i < a->ncodecs; i++) {
> > +        if (a->codecs[i]->type != b->codecs[i]->type)
> > +            return false;
> > +    }
> > +
> > +    if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> > +        !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info))
> > +        return false;
> > +
> > +    return true;
> > +}
> > +
> > +
> > +ssize_t
> > +virDomainSoundDefFind(const virDomainDef *def,
> > +                      const virDomainSoundDef *sound)
> > +{
> > +    size_t i;
> > +
> > +    for (i = 0; i < def->nsounds; i++) {
> > +        if (virDomainSoundDefEquals(sound, def->sounds[i]))
> > +            return i;
> > +    }
> > +
> > +    return -1;
> > +}
> > +
> > +
> >   static virDomainWatchdogDefPtr
> >   virDomainWatchdogDefParseXML(virDomainXMLOptionPtr xmlopt,
> >                                xmlNodePtr node,
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 49cb697..cdb7b92 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -2883,7 +2883,10 @@ void virDomainChrDefFree(virDomainChrDefPtr def);
> >   int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
> >                                 virDomainChrSourceDefPtr src);
> >   void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def);
> > +ssize_t virDomainSoundDefFind(const virDomainDef *def,
> > +                              const virDomainSoundDef *sound);
> >   void virDomainSoundDefFree(virDomainSoundDefPtr def);
> > +virDomainSoundDefPtr virDomainSoundDefRemove(virDomainDefPtr def, size_t idx);
> >   void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def);
> >   void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
> >   void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 8fe0bf9..cdbd64f 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -578,7 +578,9 @@ virDomainSmartcardTypeFromString;
> >   virDomainSmartcardTypeToString;
> >   virDomainSmbiosModeTypeFromString;
> >   virDomainSmbiosModeTypeToString;
> > +virDomainSoundDefFind;
> >   virDomainSoundDefFree;
> > +virDomainSoundDefRemove;
> >   virDomainSoundModelTypeFromString;
> >   virDomainSoundModelTypeToString;
> >   virDomainStartupPolicyTypeFromString;
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 18bd010..8d91bb2 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -8302,6 +8302,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> >   {
> >       virDomainDiskDefPtr disk, det_disk;
> >       virDomainNetDefPtr net;
> > +    virDomainSoundDefPtr sound;
> >       virDomainHostdevDefPtr hostdev, det_hostdev;
> >       virDomainLeaseDefPtr lease, det_lease;
> >       virDomainControllerDefPtr cont, det_cont;
> > @@ -8330,6 +8331,13 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> >           virDomainNetDefFree(virDomainNetRemove(vmdef, idx));
> >           break;
> >
> > +    case VIR_DOMAIN_DEVICE_SOUND:
> > +        sound = dev->data.sound;
> > +        if ((idx = virDomainSoundDefFind(vmdef, sound)) < 0)
> > +            return -1;
> > +        virDomainSoundDefFree(virDomainSoundDefRemove(vmdef, idx));
> > +        break;
> > +
> This is missing a virReportError call, as virDomainSoundDefFind doesn't
> raise one. I squashed this in and pushed:
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f72ca3b949..abd400cfee 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8339,8 +8339,11 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
>
>       case VIR_DOMAIN_DEVICE_SOUND:
>           sound = dev->data.sound;
> -        if ((idx = virDomainSoundDefFind(vmdef, sound)) < 0)
> +        if ((idx = virDomainSoundDefFind(vmdef, sound)) < 0) {
> +            virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> +                           _("device not present in domain
> configuration"));
>               return -1;
> +        }
>           virDomainSoundDefFree(virDomainSoundDefRemove(vmdef, idx));
>           break;
ok,I understand and support it.
> Thanks,
> Cole
Thanks,

Jidong Xia




--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list