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@cmss.chinamobile.com>
---
src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 2 ++
src/qemu/qemu_driver.c | 9 ++++++-
4 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c1705a0..4083b7c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2754,6 +2754,15 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr 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 virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
{
if (!def)
@@ -17211,6 +17220,58 @@ virDomainNetUpdate(virDomainDefPtr def,
return 0;
}
+/**
+ * virDomainSoundFindIdx:
+ * @def: domain definition
+ * @sound: sound definition
+ *
+ * Lookup domain's sound interface based on passed @sound
+ * definition.
+ *
+ * Return: index of match if unique match found,
+ * -1 otherwise and an error is logged.
+ */
+
+int
+virDomainSoundFindIdx(virDomainDefPtr def, virDomainSoundDefPtr sound)
+{
+ size_t i;
+ int matchidx = -1;
+ bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&sound->info,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
+
+ for (i = 0; i < def->nsounds; i++) {
+
+ if (PCIAddrSpecified &&
+ !virPCIDeviceAddressEqual(&def->sounds[i]->info.addr.pci,
+ &sound->info.addr.pci))
+ continue;
+
+ if (matchidx >= 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("multiple matching devices found"));
+
+ return -1;
+ }
+
+ matchidx = i;
+ }
+
+ if (matchidx < 0) {
+ if (PCIAddrSpecified) {
+ virReportError(VIR_ERR_DEVICE_MISSING,
+ _("no device found on %.4x:%.2x:%.2x.%.1x"),
+ sound->info.addr.pci.domain,
+ sound->info.addr.pci.bus,
+ sound->info.addr.pci.slot,
+ sound->info.addr.pci.function);
+ } else {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("no matching device found"));
+ }
+ }
+ return matchidx;
+}
int virDomainControllerInsert(virDomainDefPtr def,
virDomainControllerDefPtr controller)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f7404b8..676c076 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2873,6 +2873,8 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def);
void virDomainChrDefFree(virDomainChrDefPtr def);
int virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
virDomainChrSourceDefPtr src);
+int virDomainSoundFindIdx(virDomainDefPtr def, virDomainSoundDefPtr sound);
+virDomainSoundDefPtr virDomainSoundDefRemove(virDomainDefPtr def, size_t idx);
void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def);
void virDomainSoundDefFree(virDomainSoundDefPtr def);
void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7d62df8..c2d9175 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -579,6 +579,8 @@ virDomainSmartcardTypeToString;
virDomainSmbiosModeTypeFromString;
virDomainSmbiosModeTypeToString;
virDomainSoundDefFree;
+virDomainSoundDefRemove;
+virDomainSoundFindIdx;
virDomainSoundModelTypeFromString;
virDomainSoundModelTypeToString;
virDomainStartupPolicyTypeFromString;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e7156c..3281bca 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8476,6 +8476,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
{
virDomainDiskDefPtr disk, det_disk;
virDomainNetDefPtr net;
+ virDomainSoundDefPtr sound;
virDomainHostdevDefPtr hostdev, det_hostdev;
virDomainLeaseDefPtr lease, det_lease;
virDomainControllerDefPtr cont, det_cont;
@@ -8504,6 +8505,13 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainNetDefFree(virDomainNetRemove(vmdef, idx));
break;
+ case VIR_DOMAIN_DEVICE_SOUND:
+ sound = dev->data.sound;
+ if ((idx = virDomainSoundFindIdx(vmdef, sound)) < 0)
+ return -1;
+ virDomainSoundDefFree(virDomainSoundDefRemove(vmdef, idx));
+ break;
+
case VIR_DOMAIN_DEVICE_HOSTDEV: {
hostdev = dev->data.hostdev;
if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) {
@@ -8634,7 +8642,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
vmdef->vsock = NULL;
break;
- case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 10/15/19 2:41 AM, Jidong Xia wrote:
> With this patch users can cold unplug some sound devices.
> use "virsh detach-device vm sound.xml --config" command.
>
I reviewed and pushed patch #1
Some comments below
> Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com>
> ---
> src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 2 ++
> src/libvirt_private.syms | 2 ++
> src/qemu/qemu_driver.c | 9 ++++++-
> 4 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c1705a0..4083b7c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2754,6 +2754,15 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr 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 virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
> {
> if (!def)
> @@ -17211,6 +17220,58 @@ virDomainNetUpdate(virDomainDefPtr def,
> return 0;
> }
>
> +/**
> + * virDomainSoundFindIdx:
> + * @def: domain definition
> + * @sound: sound definition
> + *
> + * Lookup domain's sound interface based on passed @sound
> + * definition.
> + *
> + * Return: index of match if unique match found,
> + * -1 otherwise and an error is logged.
> + */
> +
> +int
> +virDomainSoundFindIdx(virDomainDefPtr def, virDomainSoundDefPtr sound)
> +{
> + size_t i;
> + int matchidx = -1;
> + bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&sound->info,
> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
> +
> + for (i = 0; i < def->nsounds; i++) {
> +
> + if (PCIAddrSpecified &&
> + !virPCIDeviceAddressEqual(&def->sounds[i]->info.addr.pci,
> + &sound->info.addr.pci))
> + continue;
> +
> + if (matchidx >= 0) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("multiple matching devices found"));
> +
> + return -1;
> + }
> +
> + matchidx = i;
> + }
> +
> + if (matchidx < 0) {
> + if (PCIAddrSpecified) {
> + virReportError(VIR_ERR_DEVICE_MISSING,
> + _("no device found on %.4x:%.2x:%.2x.%.1x"),
> + sound->info.addr.pci.domain,
> + sound->info.addr.pci.bus,
> + sound->info.addr.pci.slot,
> + sound->info.addr.pci.function);
> + } else {
> + virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> + _("no matching device found"));
> + }
> + }
> + return matchidx;
> +}
>
I think the model to follow should be virDomainInputDefFind +
virDomainInputDefEquals
* Put all the matching logic in the Equals function
* It should match ->model as well. This way a use could detach just an
XML with <sound model='FOO'/> without requiring the <address> block
* Use virDomainDeviceInfoAddressIsEqual instead of restricting it to
just PCI address. Some day maybe we support usb-audio which would have a
different address type for example
You can CC me on v2 and I'll review it
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/14/19 5:52 AM, Cole Robinson wrote:
> On 10/15/19 2:41 AM, Jidong Xia wrote:
>> With this patch users can cold unplug some sound devices.
>> use "virsh detach-device vm sound.xml --config" command.
>>
> I reviewed and pushed patch #1
>
> Some comments below
>
>> Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com>
>> ---
>> src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_conf.h | 2 ++
>> src/libvirt_private.syms | 2 ++
>> src/qemu/qemu_driver.c | 9 ++++++-
>> 4 files changed, 73 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index c1705a0..4083b7c 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2754,6 +2754,15 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr 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 virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
>> {
>> if (!def)
>> @@ -17211,6 +17220,58 @@ virDomainNetUpdate(virDomainDefPtr def,
>> return 0;
>> }
>>
>> +/**
>> + * virDomainSoundFindIdx:
>> + * @def: domain definition
>> + * @sound: sound definition
>> + *
>> + * Lookup domain's sound interface based on passed @sound
>> + * definition.
>> + *
>> + * Return: index of match if unique match found,
>> + * -1 otherwise and an error is logged.
>> + */
>> +
>> +int
>> +virDomainSoundFindIdx(virDomainDefPtr def, virDomainSoundDefPtr sound)
>> +{
>> + size_t i;
>> + int matchidx = -1;
>> + bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&sound->info,
>> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
>> +
>> + for (i = 0; i < def->nsounds; i++) {
>> +
>> + if (PCIAddrSpecified &&
>> + !virPCIDeviceAddressEqual(&def->sounds[i]->info.addr.pci,
>> + &sound->info.addr.pci))
>> + continue;
>> +
>> + if (matchidx >= 0) {
>> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> + _("multiple matching devices found"));
>> +
>> + return -1;
>> + }
>> +
>> + matchidx = i;
>> + }
>> +
>> + if (matchidx < 0) {
>> + if (PCIAddrSpecified) {
>> + virReportError(VIR_ERR_DEVICE_MISSING,
>> + _("no device found on %.4x:%.2x:%.2x.%.1x"),
>> + sound->info.addr.pci.domain,
>> + sound->info.addr.pci.bus,
>> + sound->info.addr.pci.slot,
>> + sound->info.addr.pci.function);
>> + } else {
>> + virReportError(VIR_ERR_DEVICE_MISSING, "%s",
>> + _("no matching device found"));
>> + }
>> + }
>> + return matchidx;
>> +}
>>
> I think the model to follow should be virDomainInputDefFind +
> virDomainInputDefEquals
>
> * Put all the matching logic in the Equals function
> * It should match ->model as well. This way a use could detach just an
> XML with <sound model='FOO'/> without requiring the <address> block
> * Use virDomainDeviceInfoAddressIsEqual instead of restricting it to
> just PCI address. Some day maybe we support usb-audio which would have a
> different address type for example
I see, I accept it.
>
> You can CC me on v2 and I'll review it
ok,I'll send it some days later.
>
> Thanks,
> Cole
>
Thanks,
Jidong Xia
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.