[libvirt] [PATCH 0/2] qemu: cold-plug and cold-unplug of sound
With this patch users can cold plug and unplug some sound devices. Jidong Xia (2): qemu: cold-plug of sound qemu: cold-unplug of sound src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 18 ++++++++++++-- 4 files changed, 81 insertions(+), 2 deletions(-) -- 1.8.3.1
With this patch users can cold plug some sound devices. use "virsh attach-device vm sound.xml --config" command. Consider the following sound.xml for a domain: <sound model='ich6'> <address type='pci' domain='0x0000' bus='0x00' slot='xxx' function='0'/> </sound> Signed-off-by: Jidong Xia <xiajidong@cmss.chinamobile.com> --- src/qemu/qemu_driver.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc0ede2..8e7156c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8266,6 +8266,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, { virDomainDiskDefPtr disk; virDomainNetDefPtr net; + virDomainSoundDefPtr sound; virDomainHostdevDefPtr hostdev; virDomainLeaseDefPtr lease; virDomainControllerDefPtr controller; @@ -8300,6 +8301,13 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, dev->data.net = NULL; break; + case VIR_DOMAIN_DEVICE_SOUND: + sound = dev->data.sound; + if (VIR_APPEND_ELEMENT(vmdef->sounds, vmdef->nsounds, sound) < 0) + return -1; + dev->data.sound = NULL; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: hostdev = dev->data.hostdev; if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) { @@ -8434,7 +8442,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, VIR_STEAL_PTR(vmdef->vsock, dev->data.vsock); 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
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
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
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
participants (2)
-
Cole Robinson -
Jidong Xia