10:02 a.m.
To some extent we have the code in, but this should enable the full support.
Michal Privoznik (9):
qemuDomainDetachDeviceConfig: make idx type of ssize_t
domain_conf: Validate redirdev after parsing
Export virDomainRedirdevDefFree
virDomainRedirdevDef: Introduce find & remove routines
virDomainDeviceInfoIterateInternal: Iterate through redirdevs too
qemuMonitorJSONAttachCharDev: Teach spicevmc
qemuDomainAttachDeviceConfig: Allow redirdev coldplug
qemuDomainDetachDeviceConfig: Allow cold unplug of redirdevs
qemuDomainRemoveDevice: Enable live redirdev detach
src/conf/domain_conf.c | 84 ++++++++++++++++++++++++++++++++++++++++----
src/conf/domain_conf.h | 4 +++
src/libvirt_private.syms | 3 ++
src/qemu/qemu_driver.c | 29 ++++++++++++---
src/qemu/qemu_hotplug.c | 76 ++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++
src/qemu/qemu_monitor_json.c | 7 ++++
tests/qemumonitorjsontest.c | 2 +-
8 files changed, 196 insertions(+), 12 deletions(-)
--
2.8.4
10:02 a.m.
New subject: [libvirt] [PATCH 1/9] qemuDomainDetachDeviceConfig: make idx type of ssize_t
The variable is used to hold the index to the device array we are
trying to remove. All the functions that set it are expecting it
to be type of ssize_t instead of int.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e70d3ce..d7b65f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7951,7 +7951,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainControllerDefPtr cont, det_cont;
virDomainChrDefPtr chr;
virDomainFSDefPtr fs;
- int idx;
+ ssize_t idx;
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
--
2.8.4
2:49 a.m.
New subject: [libvirt] [PATCH 1/9] qemuDomainDetachDeviceConfig: make idx type of ssize_t
On Thu, Jun 09, 2016 at 17:02:36 +0200, Michal Privoznik wrote:
The variable is used to hold the index to the device array we are
trying to remove. All the functions that set it are expecting it
to be type of ssize_t instead of int.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e70d3ce..d7b65f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7951,7 +7951,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainControllerDefPtr cont, det_cont;
virDomainChrDefPtr chr;
virDomainFSDefPtr fs;
- int idx;
+ ssize_t idx;
A random sample of the "vir*FindBy*" return an integer that is fed to
idx and then the functions removing that take a size_t. Use of int here
is consistent with the return value of those functions.
Are you planing on fixing those too?
8:14 a.m.
New subject: [libvirt] [PATCH 1/9] qemuDomainDetachDeviceConfig: make idx type of ssize_t
On 10.06.2016 09:49, Peter Krempa wrote:
On Thu, Jun 09, 2016 at 17:02:36 +0200, Michal Privoznik wrote:
> The variable is used to hold the index to the device array we are
> trying to remove. All the functions that set it are expecting it
> to be type of ssize_t instead of int.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e70d3ce..d7b65f1 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7951,7 +7951,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
> virDomainControllerDefPtr cont, det_cont;
> virDomainChrDefPtr chr;
> virDomainFSDefPtr fs;
> - int idx;
> + ssize_t idx;
A random sample of the "vir*FindBy*" return an integer that is fed to
idx and then the functions removing that take a size_t. Use of int here
is consistent with the return value of those functions.
Are you planing on fixing those too?
Ah, okay; That calls for a bigger fix and thus a separate patch set.
I'll drop this one for now then.
Michal
7:02 a.m.
New subject: [libvirt] [PATCH 1/9] qemuDomainDetachDeviceConfig: make idx type of ssize_t
On 06/09/2016 11:02 AM, Michal Privoznik wrote:
The variable is used to hold the index to the device array we are
trying to remove. All the functions that set it are expecting it
to be type of ssize_t instead of int.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e70d3ce..d7b65f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7951,7 +7951,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainControllerDefPtr cont, det_cont;
virDomainChrDefPtr chr;
virDomainFSDefPtr fs;
- int idx;
+ ssize_t idx;
If you do this, then this code that uses idx will never detect a "not
found" condition:
if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
return -1;
(there are several other similar uses in the same function)
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
7:11 a.m.
New subject: [libvirt] [PATCH 1/9] qemuDomainDetachDeviceConfig: make idx type of ssize_t
On 06/10/2016 08:02 AM, Laine Stump wrote:
On 06/09/2016 11:02 AM, Michal Privoznik wrote:
> The variable is used to hold the index to the device array we are
> trying to remove. All the functions that set it are expecting it
> to be type of ssize_t instead of int.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e70d3ce..d7b65f1 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7951,7 +7951,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr
> vmdef,
> virDomainControllerDefPtr cont, det_cont;
> virDomainChrDefPtr chr;
> virDomainFSDefPtr fs;
> - int idx;
> + ssize_t idx;
If you do this, then this code that uses idx will never detect a "not
found" condition:
if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
return -1;
(there are several other similar uses in the same function)
Ugh. Nevermind. I looked at "ssize_t" and saw "size_t". Too early, not
enough caffeine.
10:02 a.m.
New subject: [libvirt] [PATCH 2/9] domain_conf: Validate redirdev after parsing
There's currently just one limitation: redirdevs that want to go
on USB bus require a USB controller, surprisingly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 85f6e31..8f5935c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4567,14 +4567,39 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
return 0;
}
+static int
+virDomainRedirdevDefValidate(const virDomainDef *def,
+ const virDomainRedirdevDef *redirdev)
+{
+ size_t i;
+
+ if (redirdev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB)
+ return 0;
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr controller = def->controllers[i];
+
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can't add redirected USB device: "
+ "USB is disabled for this domain"));
+ }
+ }
+
+ return 0;
+}
+
static int
virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
- const virDomainDef *def ATTRIBUTE_UNUSED)
+ const virDomainDef *def)
{
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
return virDomainDiskDefValidate(dev->data.disk);
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ return virDomainRedirdevDefValidate(def, dev->data.redirdev);
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_NET:
@@ -4586,7 +4611,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_CONTROLLER:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
@@ -17060,10 +17084,7 @@ virDomainDefParseXML(xmlDocPtr xml,
if (!redirdev)
goto error;
- if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && usb_none) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Can't add redirected USB device: "
- "USB is disabled for this domain"));
+ if (virDomainRedirdevDefValidate(def, redirdev) < 0) {
virDomainRedirdevDefFree(redirdev);
goto error;
}
--
2.8.4
2:33 a.m.
New subject: [libvirt] [PATCH 2/9] domain_conf: Validate redirdev after parsing
On Thu, Jun 09, 2016 at 17:02:37 +0200, Michal Privoznik wrote:
There's currently just one limitation: redirdevs that want to go
on USB bus require a USB controller, surprisingly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 85f6e31..8f5935c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4567,14 +4567,39 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
return 0;
}
+static int
+virDomainRedirdevDefValidate(const virDomainDef *def,
+ const virDomainRedirdevDef *redirdev)
+{
+ size_t i;
+
+ if (redirdev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB)
This condition makes it harder to add checks for non-usb redirdevs
later.
+ return 0;
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr controller = def->controllers[i];
+
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
We already have a helper to do this called 'virDomainDefHasUSB'
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s",
+ _("Can't add redirected USB device: "
+ "USB is disabled for this domain"));
+ }
+ }
+
+ return 0;
+}
+
static int
virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
- const virDomainDef *def ATTRIBUTE_UNUSED)
+ const virDomainDef *def)
{
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
return virDomainDiskDefValidate(dev->data.disk);
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ return virDomainRedirdevDefValidate(def, dev->data.redirdev);
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_NET:
@@ -4586,7 +4611,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_CONTROLLER:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
@@ -17060,10 +17084,7 @@ virDomainDefParseXML(xmlDocPtr xml,
if (!redirdev)
goto error;
- if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && usb_none) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Can't add redirected USB device: "
- "USB is disabled for this domain"));
+ if (virDomainRedirdevDefValidate(def, redirdev) < 0) {
Ideally this would be removed altogether. Such checks shouldn't be in
the parser.
virDomainRedirdevDefFree(redirdev);
goto error;
}
10:02 a.m.
New subject: [libvirt] [PATCH 3/9] Export virDomainRedirdevDefFree
In the 162efa1a commit the function was introduced, but the
commit forgot to update livirt_private.syms accordingly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 42f664c..fe7a34d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -424,6 +424,7 @@ virDomainPMSuspendedReasonTypeFromString;
virDomainPMSuspendedReasonTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
+virDomainRedirdevDefFree;
virDomainRNGBackendTypeToString;
virDomainRNGDefFree;
virDomainRNGFind;
--
2.8.4
2:50 a.m.
New subject: [libvirt] [PATCH 3/9] Export virDomainRedirdevDefFree
On Thu, Jun 09, 2016 at 17:02:38 +0200, Michal Privoznik wrote:
In the 162efa1a commit the function was introduced, but the
commit forgot to update livirt_private.syms accordingly.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
1 file changed, 1 insertion(+)
ACK
10:02 a.m.
New subject: [libvirt] [PATCH 4/9] virDomainRedirdevDef: Introduce find & remove routines
Basically, there are just two functions introduced here:
virDomainRedirdevDefFind which looks up given redirdev in domain
definition, and virDomainRedirdevDefRemove which removes the
device at given index in the array of devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 4 ++++
src/libvirt_private.syms | 2 ++
3 files changed, 50 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f5935c..7b2ff98 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14597,6 +14597,50 @@ virDomainMemoryRemove(virDomainDefPtr def,
}
+ssize_t
+virDomainRedirdevDefFind(virDomainDefPtr def,
+ virDomainRedirdevDefPtr redirdev)
+{
+ size_t i;
+
+ for (i = 0; i < def->nredirdevs; i++) {
+ virDomainRedirdevDefPtr tmp = def->redirdevs[i];
+
+ if (redirdev->bus != tmp->bus)
+ continue;
+
+ if (!virDomainChrSourceDefIsEqual(&redirdev->source.chr,
+ &tmp->source.chr))
+ continue;
+
+ if (redirdev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virDomainDeviceInfoAddressIsEqual(&redirdev->info,
&tmp->info))
+ continue;
+
+ if (STRNEQ_NULLABLE(redirdev->info.alias, tmp->info.alias))
+ continue;
+
+ break;
+ }
+
+ if (i < def->nredirdevs)
+ return i;
+
+ return -1;
+}
+
+
+virDomainRedirdevDefPtr
+virDomainRedirdevDefRemove(virDomainDefPtr def, size_t idx)
+{
+ virDomainRedirdevDefPtr ret = def->redirdevs[idx];
+
+ VIR_DELETE_ELEMENT(def->redirdevs, idx, def->nredirdevs);
+
+ return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3792562..c1b002a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2827,6 +2827,10 @@ virDomainChrRemove(virDomainDefPtr vmdef,
ssize_t virDomainRNGFind(virDomainDefPtr def, virDomainRNGDefPtr rng);
virDomainRNGDefPtr virDomainRNGRemove(virDomainDefPtr def, size_t idx);
+ssize_t virDomainRedirdevDefFind(virDomainDefPtr def,
+ virDomainRedirdevDefPtr redirdev);
+virDomainRedirdevDefPtr virDomainRedirdevDefRemove(virDomainDefPtr def, size_t idx);
+
int virDomainSaveXML(const char *configDir,
virDomainDefPtr def,
const char *xml);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe7a34d..fc401cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -424,7 +424,9 @@ virDomainPMSuspendedReasonTypeFromString;
virDomainPMSuspendedReasonTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
+virDomainRedirdevDefFind;
virDomainRedirdevDefFree;
+virDomainRedirdevDefRemove;
virDomainRNGBackendTypeToString;
virDomainRNGDefFree;
virDomainRNGFind;
--
2.8.4
3:06 a.m.
New subject: [libvirt] [PATCH 4/9] virDomainRedirdevDef: Introduce find & remove routines
On Thu, Jun 09, 2016 at 17:02:39 +0200, Michal Privoznik wrote:
Basically, there are just two functions introduced here:
virDomainRedirdevDefFind which looks up given redirdev in domain
definition, and virDomainRedirdevDefRemove which removes the
device at given index in the array of devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 4 ++++
src/libvirt_private.syms | 2 ++
3 files changed, 50 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f5935c..7b2ff98 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14597,6 +14597,50 @@ virDomainMemoryRemove(virDomainDefPtr def,
}
+ssize_t
+virDomainRedirdevDefFind(virDomainDefPtr def,
+ virDomainRedirdevDefPtr redirdev)
+{
+ size_t i;
+
+ for (i = 0; i < def->nredirdevs; i++) {
+ virDomainRedirdevDefPtr tmp = def->redirdevs[i];
+
+ if (redirdev->bus != tmp->bus)
+ continue;
+
+ if (!virDomainChrSourceDefIsEqual(&redirdev->source.chr,
+ &tmp->source.chr))
+ continue;
+
+ if (redirdev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+ !virDomainDeviceInfoAddressIsEqual(&redirdev->info,
&tmp->info))
+ continue;
+
+ if (STRNEQ_NULLABLE(redirdev->info.alias, tmp->info.alias))
+ continue;
I don't think we currently match the alias across our device finding
functions. The reason for that is that you won't be able to unplug the
device with --live --config specified for virsh or the corresponding
flag values as the inactive device doesn't have an alias.
+
+ break;
You can "return i" right away since you already found the device.
+ }
+
+ if (i < def->nredirdevs)
+ return i;
This is then not necessary
+
+ return -1;
+}
3:30 a.m.
New subject: [libvirt] [PATCH 4/9] virDomainRedirdevDef: Introduce find & remove routines
On 10.06.2016 10:06, Peter Krempa wrote:
On Thu, Jun 09, 2016 at 17:02:39 +0200, Michal Privoznik wrote:
> Basically, there are just two functions introduced here:
> virDomainRedirdevDefFind which looks up given redirdev in domain
> definition, and virDomainRedirdevDefRemove which removes the
> device at given index in the array of devices.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 4 ++++
> src/libvirt_private.syms | 2 ++
> 3 files changed, 50 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8f5935c..7b2ff98 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14597,6 +14597,50 @@ virDomainMemoryRemove(virDomainDefPtr def,
> }
>
>
> +ssize_t
> +virDomainRedirdevDefFind(virDomainDefPtr def,
> + virDomainRedirdevDefPtr redirdev)
> +{
> + size_t i;
> +
> + for (i = 0; i < def->nredirdevs; i++) {
> + virDomainRedirdevDefPtr tmp = def->redirdevs[i];
> +
> + if (redirdev->bus != tmp->bus)
> + continue;
> +
> + if (!virDomainChrSourceDefIsEqual(&redirdev->source.chr,
> + &tmp->source.chr))
> + continue;
> +
> + if (redirdev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
&&
> + !virDomainDeviceInfoAddressIsEqual(&redirdev->info,
&tmp->info))
> + continue;
> +
> + if (STRNEQ_NULLABLE(redirdev->info.alias, tmp->info.alias))
> + continue;
I don't think we currently match the alias across our device finding
functions. The reason for that is that you won't be able to unplug the
device with --live --config specified for virsh or the corresponding
flag values as the inactive device doesn't have an alias.
The problem here is that in some cases alias is the only way of
differentiating two redirdevs:
<redirdev bus='usb' type='spicevmc'>
<alias name='redir0'/>
</redirdev>
<redirdev bus='usb' type='spicevmc'>
<alias name='redir1'/>
</redirdev>
There is no address, nothing. So what do you suggest to do in this case?
I came up with comparing aliases.
Michal
3:42 a.m.
New subject: [libvirt] [PATCH 4/9] virDomainRedirdevDef: Introduce find & remove routines
On Fri, Jun 10, 2016 at 10:30:29 +0200, Michal Privoznik wrote:
On 10.06.2016 10:06, Peter Krempa wrote:
> On Thu, Jun 09, 2016 at 17:02:39 +0200, Michal Privoznik wrote:
>> Basically, there are just two functions introduced here:
>> virDomainRedirdevDefFind which looks up given redirdev in domain
>> definition, and virDomainRedirdevDefRemove which removes the
>> device at given index in the array of devices.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_conf.h | 4 ++++
>> src/libvirt_private.syms | 2 ++
>> 3 files changed, 50 insertions(+)
[...]
> I don't think we currently match the alias across our device
finding
> functions. The reason for that is that you won't be able to unplug the
> device with --live --config specified for virsh or the corresponding
> flag values as the inactive device doesn't have an alias.
The problem here is that in some cases alias is the only way of
differentiating two redirdevs:
<redirdev bus='usb' type='spicevmc'>
<alias name='redir0'/>
</redirdev>
<redirdev bus='usb' type='spicevmc'>
<alias name='redir1'/>
</redirdev>
There is no address, nothing. So what do you suggest to do in this case?
I came up with comparing aliases.
If there's no difference between them, then just pick one at random. Or
use similar logic as in virDomainMemoryFindByDefInternal where we match
the address only if it was provided by the user. Otherwise we pick one
at random from those matching the rest of the data.
10:02 a.m.
New subject: [libvirt] [PATCH 5/9] virDomainDeviceInfoIterateInternal: Iterate through redirdevs too
This is going to be important later when we received
DEVICE_DELETED event on the qemu monitor. If we do,
virDomainDefFindDevice() is called to find the device for given
device alias in the virDomainDef tree. When we enable removal for
redirdevs we need to include them in the lookup process too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7b2ff98..32c3460 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3503,6 +3503,13 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
return -1;
}
+ device.type = VIR_DOMAIN_DEVICE_REDIRDEV;
+ for (i = 0; i < def->nredirdevs; i++) {
+ device.data.redirdev = def->redirdevs[i];
+ if (cb(def, &device, &def->redirdevs[i]->info, opaque) < 0)
+ return -1;
+ }
+
/* Coverity is not very happy with this - all dead_error_condition */
#if !STATIC_ANALYSIS
/* This switch statement is here to trigger compiler warning when adding
--
2.8.4
3:52 a.m.
New subject: [libvirt] [PATCH 5/9] virDomainDeviceInfoIterateInternal: Iterate through redirdevs too
On Thu, Jun 09, 2016 at 17:02:40 +0200, Michal Privoznik wrote:
This is going to be important later when we received
DEVICE_DELETED event on the qemu monitor. If we do,
virDomainDefFindDevice() is called to find the device for given
device alias in the virDomainDef tree. When we enable removal for
redirdevs we need to include them in the lookup process too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7b2ff98..32c3460 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3503,6 +3503,13 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
return -1;
}
You need to check here that the VM has USB controllers defined.
Otherwise loading of the resulting XML file will fail as there's a check
in the parser.
+ device.type = VIR_DOMAIN_DEVICE_REDIRDEV;
+ for (i = 0; i < def->nredirdevs; i++) {
+ device.data.redirdev = def->redirdevs[i];
+ if (cb(def, &device, &def->redirdevs[i]->info, opaque) < 0)
+ return -1;
+ }
3:56 a.m.
New subject: [libvirt] [PATCH 5/9] virDomainDeviceInfoIterateInternal: Iterate through redirdevs too
On Fri, Jun 10, 2016 at 10:52:47 +0200, Peter Krempa wrote:
On Thu, Jun 09, 2016 at 17:02:40 +0200, Michal Privoznik wrote:
> This is going to be important later when we received
> DEVICE_DELETED event on the qemu monitor. If we do,
> virDomainDefFindDevice() is called to find the device for given
> device alias in the virDomainDef tree. When we enable removal for
> redirdevs we need to include them in the lookup process too.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7b2ff98..32c3460 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3503,6 +3503,13 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
> return -1;
> }
You need to check here that the VM has USB controllers defined.
Otherwise loading of the resulting XML file will fail as there's a check
in the parser.
Ignore this, this belongs to a completely different patch.
>
> + device.type = VIR_DOMAIN_DEVICE_REDIRDEV;
> + for (i = 0; i < def->nredirdevs; i++) {
> + device.data.redirdev = def->redirdevs[i];
> + if (cb(def, &device, &def->redirdevs[i]->info, opaque) <
0)
> + return -1;
> + }
ACK
10:02 a.m.
New subject: [libvirt] [PATCH 6/9] qemuMonitorJSONAttachCharDev: Teach spicevmc
We have the code for attaching redirdevs for ages now.
Unfortunately, our monitor code that handles talking to the qemu
process was missing a little piece of code that actually enabled
the feature.
BTW: it really is called "type" on the monitor, even though it's
called "name" on the cmd line. Don't ask.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 7 +++++++
tests/qemumonitorjsontest.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 12d2e22..380ddab 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6209,6 +6209,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ backend_type = "spicevmc";
+
+ if (virJSONValueObjectAppendString(data, "type",
+
virDomainChrSpicevmcTypeToString(chr->data.spicevmc)) < 0)
+ goto error;
+ break;
+
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
case VIR_DOMAIN_CHR_TYPE_PIPE:
case VIR_DOMAIN_CHR_TYPE_STDIO:
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2cd824f..0bcf62a 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -769,7 +769,7 @@ testQemuMonitorJSONAttachChardev(const void *data)
CHECK("chr_unix", "{\"return\": {}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_SPICEVMC };
- CHECK_FAIL("chr_spicevmc", "{\"return\": {}}");
+ CHECK("chr_spicevmc", "{\"return\": {}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PIPE };
CHECK_FAIL("chr_pipe", "{\"return\": {}}");
--
2.8.4
3:55 a.m.
New subject: [libvirt] [PATCH 6/9] qemuMonitorJSONAttachCharDev: Teach spicevmc
On Thu, Jun 09, 2016 at 17:02:41 +0200, Michal Privoznik wrote:
We have the code for attaching redirdevs for ages now.
Unfortunately, our monitor code that handles talking to the qemu
process was missing a little piece of code that actually enabled
the feature.
BTW: it really is called "type" on the monitor, even though it's
called "name" on the cmd line. Don't ask.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 7 +++++++
tests/qemumonitorjsontest.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
ACK
10:02 a.m.
New subject: [libvirt] [PATCH 7/9] qemuDomainAttachDeviceConfig: Allow redirdev coldplug
This is really simple, we just need to append the device into the
domain def and that's it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d7b65f1..0baf2c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7792,6 +7792,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainLeaseDefPtr lease;
virDomainControllerDefPtr controller;
virDomainFSDefPtr fs;
+ virDomainRedirdevDefPtr redirdev;
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -7909,6 +7910,14 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ redirdev = dev->data.redirdev;
+
+ if (VIR_APPEND_ELEMENT(vmdef->redirdevs, vmdef->nredirdevs, redirdev) <
0)
+ return -1;
+ dev->data.redirdev = NULL;
+ break;
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -7919,7 +7928,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
case VIR_DOMAIN_DEVICE_SHMEM:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
--
2.8.4
3:57 a.m.
New subject: [libvirt] [PATCH 7/9] qemuDomainAttachDeviceConfig: Allow redirdev coldplug
On Thu, Jun 09, 2016 at 17:02:42 +0200, Michal Privoznik wrote:
This is really simple, we just need to append the device into the
domain def and that's it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d7b65f1..0baf2c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
[...]
@@ -7909,6 +7910,14 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr
vmdef,
dev->data.memory = NULL;
break;
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ redirdev = dev->data.redirdev;
+
You need to check here that the VM has USB controllers defined.
Otherwise loading of the resulting XML file will fail as there's a check
in the parser.
+ if (VIR_APPEND_ELEMENT(vmdef->redirdevs,
vmdef->nredirdevs, redirdev) < 0)
+ return -1;
+ dev->data.redirdev = NULL;
+ break;
8:11 a.m.
New subject: [libvirt] [PATCH 7/9] qemuDomainAttachDeviceConfig: Allow redirdev coldplug
On 10.06.2016 10:57, Peter Krempa wrote:
On Thu, Jun 09, 2016 at 17:02:42 +0200, Michal Privoznik wrote:
> This is really simple, we just need to append the device into the
> domain def and that's it.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d7b65f1..0baf2c8 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
[...]
> @@ -7909,6 +7910,14 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
> dev->data.memory = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_REDIRDEV:
> + redirdev = dev->data.redirdev;
> +
You need to check here that the VM has USB controllers defined.
Otherwise loading of the resulting XML file will fail as there's a check
in the parser.
I don't think I need to. I mean, qemuDomainAttachDeviceConfig() is
called only from qemuDomainAttachDeviceFlags(). Here, user given XML is
parsed by virDomainDeviceDefParse() which at its very end calls
virDomainDeviceDefValidate() which in turn calls
virDomainDeviceDefValidateInternal(). That brings us to patch 02/09
where I'm changing virDomainDeviceDefValidateInternal() so that it
validates redirdevs too.
Or have I missed something?
Michal
8:15 a.m.
New subject: [libvirt] [PATCH 7/9] qemuDomainAttachDeviceConfig: Allow redirdev coldplug
On Fri, Jun 10, 2016 at 15:11:02 +0200, Michal Privoznik wrote:
On 10.06.2016 10:57, Peter Krempa wrote:
> On Thu, Jun 09, 2016 at 17:02:42 +0200, Michal Privoznik wrote:
>> This is really simple, we just need to append the device into the
>> domain def and that's it.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index d7b65f1..0baf2c8 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>
> [...]
>
>> @@ -7909,6 +7910,14 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
>> dev->data.memory = NULL;
>> break;
>>
>> + case VIR_DOMAIN_DEVICE_REDIRDEV:
>> + redirdev = dev->data.redirdev;
>> +
>
> You need to check here that the VM has USB controllers defined.
> Otherwise loading of the resulting XML file will fail as there's a check
> in the parser.
I don't think I need to. I mean, qemuDomainAttachDeviceConfig() is
called only from qemuDomainAttachDeviceFlags(). Here, user given XML is
parsed by virDomainDeviceDefParse() which at its very end calls
virDomainDeviceDefValidate() which in turn calls
virDomainDeviceDefValidateInternal(). That brings us to patch 02/09
where I'm changing virDomainDeviceDefValidateInternal() so that it
validates redirdevs too.
Or have I missed something?
Erm. Apparently I forgot about the patches that I've pushed a few days
ago.
10:02 a.m.
New subject: [libvirt] [PATCH 8/9] qemuDomainDetachDeviceConfig: Allow cold unplug of redirdevs
This is fairly simple. We lookup the device in the array of
devices and remove it. No magic.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0baf2c8..406ce45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8060,6 +8060,18 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx));
break;
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ if ((idx = virDomainRedirdevDefFind(vmdef,
+ dev->data.redirdev)) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("no matching redirdev was not found"));
+ return -1;
+ }
+
+ virDomainRedirdevDefFree(virDomainRedirdevDefRemove(vmdef, idx));
+ break;
+
+
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_VIDEO:
@@ -8070,7 +8082,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
case VIR_DOMAIN_DEVICE_SHMEM:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
--
2.8.4
3:58 a.m.
New subject: [libvirt] [PATCH 8/9] qemuDomainDetachDeviceConfig: Allow cold unplug of redirdevs
On Thu, Jun 09, 2016 at 17:02:43 +0200, Michal Privoznik wrote:
This is fairly simple. We lookup the device in the array of
devices and remove it. No magic.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0baf2c8..406ce45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
ACK even with necessary tweaks to accomodate prior comments.
10:02 a.m.
New subject: [libvirt] [PATCH 9/9] qemuDomainRemoveDevice: Enable live redirdev detach
For some reason, as soon as redirdev is detached, qemu removes
the chardev too. Well, it's easier for us then, so hard feelings
here.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 4 ++-
src/qemu/qemu_hotplug.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++
3 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 406ce45..8b27a08 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7637,6 +7637,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_MEMORY:
ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ ret = qemuDomainDetachRedirdevDevice(driver, vm, dev->data.redirdev);
+ break;
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -7649,7 +7652,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
case VIR_DOMAIN_DEVICE_SHMEM:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 58156c6..a7a3675 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3230,6 +3230,34 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
return ret;
}
+static int
+qemuDomainRemoveRedirdevDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRedirdevDefPtr redirdev)
+{
+ virObjectEventPtr event;
+ ssize_t idx;
+
+ VIR_DEBUG("Removing redirdev device %s from domain %p %s",
+ redirdev->info.alias, vm, vm->def->name);
+
+ if ((idx = virDomainRedirdevDefFind(vm->def, redirdev)) < 0)
+ return 0;
+
+ /* QEMU removed the chardev for us already. */
+
+ virDomainAuditRedirdev(vm, redirdev, "detach", true);
+
+ event = virDomainEventDeviceRemovedNewFromObj(vm, redirdev->info.alias);
+ qemuDomainEventQueue(driver, event);
+
+ virDomainRedirdevDefRemove(vm->def, idx);
+ qemuDomainReleaseDeviceAddress(vm, &redirdev->info, NULL);
+ virDomainRedirdevDefFree(redirdev);
+ return 0;
+}
+
+
int
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
@@ -3261,6 +3289,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_MEMORY:
ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
break;
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ ret = qemuDomainRemoveRedirdevDevice(driver, vm, dev->data.redirdev);
+ break;
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
@@ -3271,7 +3302,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_WATCHDOG:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
- case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
@@ -4172,3 +4202,47 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
qemuDomainResetDeviceRemoval(vm);
return ret;
}
+
+int
+qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRedirdevDefPtr redirdev)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainRedirdevDefPtr tmpRedirdev;
+ ssize_t idx;
+
+ if ((idx = virDomainRedirdevDefFind(vm->def, redirdev)) < 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("no matching redirdev was not found"));
+ return -1;
+ }
+
+ tmpRedirdev = vm->def->redirdevs[idx];
+
+ if (!tmpRedirdev->info.alias) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("alias for the redirdev was not found"));
+ goto cleanup;
+ }
+
+ qemuDomainMarkDeviceForRemoval(vm, &tmpRedirdev->info);
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ if (qemuMonitorDelDevice(priv->mon, tmpRedirdev->info.alias) < 0) {
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+ }
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+ qemuDomainReleaseDeviceAddress(vm, &tmpRedirdev->info, NULL);
+ ret = qemuDomainRemoveRedirdevDevice(driver, vm, tmpRedirdev);
+ }
+
+ cleanup:
+ qemuDomainResetDeviceRemoval(vm);
+ return ret;
+}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 165d345..fc79ee5 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -46,6 +46,9 @@ int qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainRedirdevDefPtr hostdev);
+int qemuDomainDetachRedirdevDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRedirdevDefPtr hostdev);
int qemuDomainAttachHostDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
--
2.8.4
10:16 a.m.
On 06/09/2016 11:02 AM, Michal Privoznik wrote:
To some extent we have the code in, but this should enable the full
support.
Michal Privoznik (9):
qemuDomainDetachDeviceConfig: make idx type of ssize_t
domain_conf: Validate redirdev after parsing
Export virDomainRedirdevDefFree
virDomainRedirdevDef: Introduce find & remove routines
virDomainDeviceInfoIterateInternal: Iterate through redirdevs too
qemuMonitorJSONAttachCharDev: Teach spicevmc
qemuDomainAttachDeviceConfig: Allow redirdev coldplug
qemuDomainDetachDeviceConfig: Allow cold unplug of redirdevs
qemuDomainRemoveDevice: Enable live redirdev detach
src/conf/domain_conf.c | 84 ++++++++++++++++++++++++++++++++++++++++----
src/conf/domain_conf.h | 4 +++
src/libvirt_private.syms | 3 ++
src/qemu/qemu_driver.c | 29 ++++++++++++---
src/qemu/qemu_hotplug.c | 76 ++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_hotplug.h | 3 ++
src/qemu/qemu_monitor_json.c | 7 ++++
tests/qemumonitorjsontest.c | 2 +-
8 files changed, 196 insertions(+), 12 deletions(-)
Note there's an upstream bug for part of this:
https://bugzilla.redhat.com/show_bug.cgi?id=1313272
- Cole
3088
days inactive
3089
days old
26 comments
4 participants
participants (4)
-
Cole Robinson -
Laine Stump -
Michal Privoznik -
Peter Krempa