1:56 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1599971
Ján Tomko (11):
qemu: move out legacy USB controller formatting
qemu: exit early if USB_CONTROLLER_MODEL_NONE is present
Add qemuBuildDomainForbidLegacyUSBController
qemu: separate counting of legacy USB controllers
qemu: separate counting of USB controllers
qemuBuildControllerDevStr: remove nusbcontroller argument
qemu: Introduce qemuBuildControllersByTypeCommandLine
qemu: format CCID controllers after USB hubs
qemuBuildControllersByTypeCommandLine: free devstr in the cleanup
section
rename qemuBuildControllerDevCommandLine
qemuBuildControllersCommandLine: use i instead of j as the counter
src/qemu/qemu_command.c | 222 +++++++++++-------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
tests/qemuxml2argvdata/chardev-reconnect.args | 2 +-
tests/qemuxml2argvdata/controller-order.args | 2 +-
tests/qemuxml2argvdata/name-escape.args | 2 +-
.../smartcard-controller.args | 2 +-
.../smartcard-host-certificates-database.args | 2 +-
.../smartcard-host-certificates.args | 2 +-
tests/qemuxml2argvdata/smartcard-host.args | 2 +-
.../smartcard-passthrough-spicevmc.args | 2 +-
.../smartcard-passthrough-tcp.args | 2 +-
tests/qemuxml2argvdata/user-aliases.args | 2 +-
13 files changed, 150 insertions(+), 97 deletions(-)
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 01/11] qemu: move out legacy USB controller formatting
Move out the code formatting "-usb" on the QEMU command line.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 822d5f8669..d6b7d11cc6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3021,6 +3021,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
}
+static int
+qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
+ const virDomainDef *def,
+ int usbcontroller)
+{
+ if (usbcontroller == 0 &&
+ !qemuDomainIsQ35(def) &&
+ !qemuDomainIsARMVirt(def) &&
+ !qemuDomainIsRISCVVirt(def) &&
+ !ARCH_IS_S390(def->os.arch)) {
+ /* We haven't added any USB controller yet, but we haven't been asked
+ * not to add one either. Add a legacy USB controller, unless we're
+ * creating a kind of guest we want to keep legacy-free */
+ virCommandAddArg(cmd, "-usb");
+ }
+
+ return 0;
+}
+
+
/**
* qemuBuildSkipController:
* @controller: Controller to check
@@ -3156,16 +3176,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
}
}
- if (usbcontroller == 0 &&
- !qemuDomainIsQ35(def) &&
- !qemuDomainIsARMVirt(def) &&
- !qemuDomainIsRISCVVirt(def) &&
- !ARCH_IS_S390(def->os.arch)) {
- /* We haven't added any USB controller yet, but we haven't been asked
- * not to add one either. Add a legacy USB controller, unless we're
- * creating a kind of guest we want to keep legacy-free */
- virCommandAddArg(cmd, "-usb");
- }
+ if (qemuBuildLegacyUSBControllerCommandLine(cmd, def, usbcontroller) < 0)
+ goto cleanup;
ret = 0;
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 02/11] qemu: exit early if USB_CONTROLLER_MODEL_NONE is present
This removes the need to mark it in the 'usbcontroller' variable.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6b7d11cc6..80b05efa03 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3026,6 +3026,19 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
const virDomainDef *def,
int usbcontroller)
{
+ size_t i;
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+
+ if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
+ continue;
+
+ /* If we have mode='none', there are no other USB controllers */
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE)
+ return 0;
+ }
+
if (usbcontroller == 0 &&
!qemuDomainIsQ35(def) &&
!qemuDomainIsARMVirt(def) &&
@@ -3127,7 +3140,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
/* skip USB controllers with type none.*/
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
- usbcontroller = -1; /* mark we don't want a controller */
continue;
}
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 03/11] Add qemuBuildDomainForbidLegacyUSBController
Shorten some long conditions.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 80b05efa03..cce5520783 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3021,6 +3021,18 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
}
+static bool
+qemuBuildDomainForbidLegacyUSBController(const virDomainDef *def)
+{
+ if (qemuDomainIsQ35(def) ||
+ qemuDomainIsARMVirt(def) ||
+ qemuDomainIsRISCVVirt(def))
+ return true;
+
+ return false;
+}
+
+
static int
qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
const virDomainDef *def,
@@ -3040,9 +3052,7 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
}
if (usbcontroller == 0 &&
- !qemuDomainIsQ35(def) &&
- !qemuDomainIsARMVirt(def) &&
- !qemuDomainIsRISCVVirt(def) &&
+ !qemuBuildDomainForbidLegacyUSBController(def) &&
!ARCH_IS_S390(def->os.arch)) {
/* We haven't added any USB controller yet, but we haven't been asked
* not to add one either. Add a legacy USB controller, unless we're
@@ -3145,9 +3155,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT &&
- !qemuDomainIsQ35(def) &&
- !qemuDomainIsARMVirt(def) &&
- !qemuDomainIsRISCVVirt(def)) {
+ !qemuBuildDomainForbidLegacyUSBController(def)) {
/* An appropriate default USB controller model should already
* have been selected in qemuDomainDeviceDefPostParse(); if
--
2.20.1
3:20 p.m.
New subject: [libvirt] [PATCH 03/11] Add qemuBuildDomainForbidLegacyUSBController
On 01/16/2019 02:56 AM, Ján Tomko wrote:
Shorten some long conditions.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 80b05efa03..cce5520783 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3021,6 +3021,18 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
}
+static bool
+qemuBuildDomainForbidLegacyUSBController(const virDomainDef *def)
+{
+ if (qemuDomainIsQ35(def) ||
+ qemuDomainIsARMVirt(def) ||
+ qemuDomainIsRISCVVirt(def))
+ return true;
+
+ return false;
+}
+
+
static int
qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
const virDomainDef *def,
@@ -3040,9 +3052,7 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
}
if (usbcontroller == 0 &&
- !qemuDomainIsQ35(def) &&
- !qemuDomainIsARMVirt(def) &&
- !qemuDomainIsRISCVVirt(def) &&
+ !qemuBuildDomainForbidLegacyUSBController(def) &&
!ARCH_IS_S390(def->os.arch)) {
From the patch context it looks like the S390 condition was missed
here.
I see the comment below explains it but this confused me for a minute
- Cole
1:56 a.m.
New subject: [libvirt] [PATCH 04/11] qemu: separate counting of legacy USB controllers
Count them in qemuBuildLegacyUSBControllerCommandLine to remove
yet another variable accessed from the loop in
qemuBuildControllerDevCommandLine.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cce5520783..014bd031a8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3039,6 +3039,7 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
int usbcontroller)
{
size_t i;
+ size_t nlegacy = 0;
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
@@ -3049,6 +3050,16 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
/* If we have mode='none', there are no other USB controllers */
if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE)
return 0;
+
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT)
+ nlegacy++;
+ }
+
+ if (nlegacy > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Multiple legacy USB controllers are "
+ "not supported"));
+ return -1;
}
if (usbcontroller == 0 &&
@@ -3109,7 +3120,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
{
size_t i, j;
int usbcontroller = 0;
- bool usblegacy = false;
int contOrder[] = {
/*
* List of controller types that we add commandline args for,
@@ -3169,13 +3179,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
* (see 548ba43028 for the full story), so we skip
* qemuBuildControllerDevStr() but we don't ultimately end
* up adding the legacy USB controller */
- if (usblegacy) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Multiple legacy USB controllers are "
- "not supported"));
- goto cleanup;
- }
- usblegacy = true;
continue;
}
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 05/11] qemu: separate counting of USB controllers
qemuBuildLegacyUSBControllerCommandLine is the only place where
we need to count the USB controllers.
Count them again instead of keeping track in a variable passed to
qemuBuildControllerDevStr.
This removes the need for another variable in the loop in
qemuBuildControllerDevCommandLine.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 014bd031a8..22f041a956 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3035,11 +3035,11 @@ qemuBuildDomainForbidLegacyUSBController(const virDomainDef *def)
static int
qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
- const virDomainDef *def,
- int usbcontroller)
+ const virDomainDef *def)
{
size_t i;
size_t nlegacy = 0;
+ size_t nusb = 0;
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
@@ -3053,6 +3053,8 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT)
nlegacy++;
+ else
+ nusb++;
}
if (nlegacy > 1) {
@@ -3062,7 +3064,7 @@ qemuBuildLegacyUSBControllerCommandLine(virCommandPtr cmd,
return -1;
}
- if (usbcontroller == 0 &&
+ if (nusb == 0 &&
!qemuBuildDomainForbidLegacyUSBController(def) &&
!ARCH_IS_S390(def->os.arch)) {
/* We haven't added any USB controller yet, but we haven't been asked
@@ -3199,7 +3201,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
}
}
- if (qemuBuildLegacyUSBControllerCommandLine(cmd, def, usbcontroller) < 0)
+ if (qemuBuildLegacyUSBControllerCommandLine(cmd, def) < 0)
goto cleanup;
ret = 0;
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 06/11] qemuBuildControllerDevStr: remove nusbcontroller argument
Now that it's no longer needed, remove the argument.
This removes the last helper variable in
qemuBuildControllerDevCommandLine.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 10 ++--------
src/qemu/qemu_command.h | 3 +--
src/qemu/qemu_hotplug.c | 2 +-
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 22f041a956..f95231666f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2833,8 +2833,7 @@ int
qemuBuildControllerDevStr(const virDomainDef *domainDef,
virDomainControllerDefPtr def,
virQEMUCapsPtr qemuCaps,
- char **devstr,
- int *nusbcontroller)
+ char **devstr)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -2913,9 +2912,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
if (qemuBuildUSBControllerDevStr(domainDef, def, qemuCaps, &buf) == -1)
goto error;
- if (nusbcontroller)
- *nusbcontroller += 1;
-
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI: {
@@ -3121,7 +3117,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps)
{
size_t i, j;
- int usbcontroller = 0;
int contOrder[] = {
/*
* List of controller types that we add commandline args for,
@@ -3184,8 +3179,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
continue;
}
- if (qemuBuildControllerDevStr(def, cont, qemuCaps,
- &devstr, &usbcontroller) < 0)
+ if (qemuBuildControllerDevStr(def, cont, qemuCaps, &devstr) < 0)
goto cleanup;
if (devstr) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1a4f1f5da4..d9f8a818b5 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -117,8 +117,7 @@ char
int qemuBuildControllerDevStr(const virDomainDef *domainDef,
virDomainControllerDefPtr def,
virQEMUCapsPtr qemuCaps,
- char **devstr,
- int *nusbcontroller);
+ char **devstr);
int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
const char *alias,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a1c3ca999b..c3d935a412 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1022,7 +1022,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller) <
0)
goto cleanup;
- if (qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, &devstr,
NULL) < 0)
+ if (qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, &devstr)
< 0)
goto cleanup;
if (!devstr)
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 07/11] qemu: Introduce qemuBuildControllersByTypeCommandLine
Now that the inner loop does not require any other variables,
it can be easily separated. Apart from reducing the indentation
level this will allow it to be called from different code paths.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 118 +++++++++++++++++++++++-----------------
1 file changed, 68 insertions(+), 50 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f95231666f..91c7ec5f7b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3111,12 +3111,77 @@ qemuBuildSkipController(const virDomainControllerDef *controller,
}
+static int
+qemuBuildControllersByTypeCommandLine(virCommandPtr cmd,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps,
+ virDomainControllerType type)
+{
+ int ret = -1;
+ size_t i;
+
+ for (i = 0; i < def->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = def->controllers[i];
+ char *devstr;
+
+ if (cont->type != type)
+ continue;
+
+ if (qemuBuildSkipController(cont, def))
+ continue;
+
+ /* skip USB controllers with type none.*/
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
+ continue;
+ }
+
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT &&
+ !qemuBuildDomainForbidLegacyUSBController(def)) {
+
+ /* An appropriate default USB controller model should already
+ * have been selected in qemuDomainDeviceDefPostParse(); if
+ * we still have no model by now, we have to fall back to the
+ * legacy USB controller.
+ *
+ * Note that we *don't* want to end up with the legacy USB
+ * controller for q35 and virt machines, so we go ahead and
+ * fail in qemuBuildControllerDevStr(); on the other hand,
+ * for s390 machines we want to ignore any USB controller
+ * (see 548ba43028 for the full story), so we skip
+ * qemuBuildControllerDevStr() but we don't ultimately end
+ * up adding the legacy USB controller */
+ continue;
+ }
+
+ if (qemuBuildControllerDevStr(def, cont, qemuCaps, &devstr) < 0)
+ goto cleanup;
+
+ if (devstr) {
+ if (qemuCommandAddExtDevice(cmd, &cont->info) < 0) {
+ VIR_FREE(devstr);
+ goto cleanup;
+ }
+
+ virCommandAddArg(cmd, "-device");
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+
static int
qemuBuildControllerDevCommandLine(virCommandPtr cmd,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
- size_t i, j;
+ size_t j;
int contOrder[] = {
/*
* List of controller types that we add commandline args for,
@@ -3144,55 +3209,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
int ret = -1;
for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) {
- for (i = 0; i < def->ncontrollers; i++) {
- virDomainControllerDefPtr cont = def->controllers[i];
- char *devstr;
-
- if (cont->type != contOrder[j])
- continue;
-
- if (qemuBuildSkipController(cont, def))
- continue;
-
- /* skip USB controllers with type none.*/
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
- continue;
- }
-
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT &&
- !qemuBuildDomainForbidLegacyUSBController(def)) {
-
- /* An appropriate default USB controller model should already
- * have been selected in qemuDomainDeviceDefPostParse(); if
- * we still have no model by now, we have to fall back to the
- * legacy USB controller.
- *
- * Note that we *don't* want to end up with the legacy USB
- * controller for q35 and virt machines, so we go ahead and
- * fail in qemuBuildControllerDevStr(); on the other hand,
- * for s390 machines we want to ignore any USB controller
- * (see 548ba43028 for the full story), so we skip
- * qemuBuildControllerDevStr() but we don't ultimately end
- * up adding the legacy USB controller */
- continue;
- }
-
- if (qemuBuildControllerDevStr(def, cont, qemuCaps, &devstr) < 0)
- goto cleanup;
-
- if (devstr) {
- if (qemuCommandAddExtDevice(cmd, &cont->info) < 0) {
- VIR_FREE(devstr);
- goto cleanup;
- }
-
- virCommandAddArg(cmd, "-device");
- virCommandAddArg(cmd, devstr);
- VIR_FREE(devstr);
- }
- }
+ if (qemuBuildControllersByTypeCommandLine(cmd, def, qemuCaps, contOrder[j]) <
0)
+ goto cleanup;
}
if (qemuBuildLegacyUSBControllerCommandLine(cmd, def) < 0)
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 08/11] qemu: format CCID controllers after USB hubs
Since they go on the USB bus, format them after USB hubs.
https://bugzilla.redhat.com/show_bug.cgi?id=1375402
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 8 +++++++-
tests/qemuxml2argvdata/chardev-reconnect.args | 2 +-
tests/qemuxml2argvdata/controller-order.args | 2 +-
tests/qemuxml2argvdata/name-escape.args | 2 +-
tests/qemuxml2argvdata/smartcard-controller.args | 2 +-
.../smartcard-host-certificates-database.args | 2 +-
tests/qemuxml2argvdata/smartcard-host-certificates.args | 2 +-
tests/qemuxml2argvdata/smartcard-host.args | 2 +-
.../qemuxml2argvdata/smartcard-passthrough-spicevmc.args | 2 +-
tests/qemuxml2argvdata/smartcard-passthrough-tcp.args | 2 +-
tests/qemuxml2argvdata/user-aliases.args | 2 +-
11 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 91c7ec5f7b..7aafe235e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3197,6 +3197,9 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
* one. Likewise, we don't do anything for the primary IDE
* controller on an i440fx machine or primary SATA on q35, but
* we do add those beyond these two exceptions.
+ *
+ * CCID controllers are formated separately after USB hubs,
+ * because they go on the USB bus.
*/
VIR_DOMAIN_CONTROLLER_TYPE_PCI,
VIR_DOMAIN_CONTROLLER_TYPE_USB,
@@ -3204,7 +3207,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
VIR_DOMAIN_CONTROLLER_TYPE_IDE,
VIR_DOMAIN_CONTROLLER_TYPE_SATA,
VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
- VIR_DOMAIN_CONTROLLER_TYPE_CCID,
};
int ret = -1;
@@ -10602,6 +10604,10 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0)
goto error;
+ if (qemuBuildControllersByTypeCommandLine(cmd, def, qemuCaps,
+ VIR_DOMAIN_CONTROLLER_TYPE_CCID) < 0)
+ goto error;
+
if (qemuBuildDisksCommandLine(cmd, def, qemuCaps) < 0)
goto error;
diff --git a/tests/qemuxml2argvdata/chardev-reconnect.args
b/tests/qemuxml2argvdata/chardev-reconnect.args
index e88e083291..baff469c7b 100644
--- a/tests/qemuxml2argvdata/chardev-reconnect.args
+++ b/tests/qemuxml2argvdata/chardev-reconnect.args
@@ -22,8 +22,8 @@ server,nowait \
-no-acpi \
-device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-chardev socket,id=charsmartcard0,path=/tmp/channel/asdf,reconnect=20 \
-device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \
-chardev socket,id=charchannel0,host=localhost,port=1234,reconnect=10 \
diff --git a/tests/qemuxml2argvdata/controller-order.args
b/tests/qemuxml2argvdata/controller-order.args
index 2477cd2e1c..6bb4942223 100644
--- a/tests/qemuxml2argvdata/controller-order.args
+++ b/tests/qemuxml2argvdata/controller-order.args
@@ -21,8 +21,8 @@ nowait \
-boot menu=off \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7 \
--device usb-ccid,id=ccid0,bus=usb.0,port=1.1 \
-device usb-hub,id=hub0,bus=usb.0,port=1 \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1.1 \
-drive file=/tmp/fdr.img,format=raw,if=none,id=drive-virtio-disk0,cache=none,\
aio=native \
-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,\
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index 58f284014c..d314ed409a 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -22,8 +22,8 @@ bar=2/monitor.sock,server,nowait \
-no-shutdown \
-no-acpi \
-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
cache=none,throttling.bps-total=5000,throttling.iops-total=6000,\
throttling.bps-total-max=10000,throttling.iops-total-max=11000,\
diff --git a/tests/qemuxml2argvdata/smartcard-controller.args
b/tests/qemuxml2argvdata/smartcard-controller.args
index 1a654cb4e1..4c5286dd87 100644
--- a/tests/qemuxml2argvdata/smartcard-controller.args
+++ b/tests/qemuxml2argvdata/smartcard-controller.args
@@ -20,7 +20,7 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
b/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
index 17fb50699a..1056f52cfa 100644
--- a/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
+++ b/tests/qemuxml2argvdata/smartcard-host-certificates-database.args
@@ -20,8 +20,8 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,\
cert3=cert3,db=/tmp/foo,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-host-certificates.args
b/tests/qemuxml2argvdata/smartcard-host-certificates.args
index 489299a475..6bfb56b94b 100644
--- a/tests/qemuxml2argvdata/smartcard-host-certificates.args
+++ b/tests/qemuxml2argvdata/smartcard-host-certificates.args
@@ -20,8 +20,8 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,\
cert3=cert3,db=/etc/pki/nssdb,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-host.args
b/tests/qemuxml2argvdata/smartcard-host.args
index 1a654cb4e1..4c5286dd87 100644
--- a/tests/qemuxml2argvdata/smartcard-host.args
+++ b/tests/qemuxml2argvdata/smartcard-host.args
@@ -20,7 +20,7 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-passthrough-spicevmc.args
b/tests/qemuxml2argvdata/smartcard-passthrough-spicevmc.args
index 8784a3ffaa..9fbc86bce7 100644
--- a/tests/qemuxml2argvdata/smartcard-passthrough-spicevmc.args
+++ b/tests/qemuxml2argvdata/smartcard-passthrough-spicevmc.args
@@ -20,8 +20,8 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-chardev spicevmc,id=charsmartcard0,name=smartcard \
-device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/smartcard-passthrough-tcp.args
b/tests/qemuxml2argvdata/smartcard-passthrough-tcp.args
index e55126c206..12c03ad4b6 100644
--- a/tests/qemuxml2argvdata/smartcard-passthrough-tcp.args
+++ b/tests/qemuxml2argvdata/smartcard-passthrough-tcp.args
@@ -20,8 +20,8 @@ server,nowait \
-rtc base=utc \
-no-shutdown \
-no-acpi \
--device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-chardev socket,id=charsmartcard0,host=127.0.0.1,port=2001,server,nowait \
-device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/user-aliases.args
b/tests/qemuxml2argvdata/user-aliases.args
index 089a7d8d4b..7fd3b4a6e1 100644
--- a/tests/qemuxml2argvdata/user-aliases.args
+++ b/tests/qemuxml2argvdata/user-aliases.args
@@ -35,9 +35,9 @@ server,nowait \
-global PIIX4_PM.disable_s3=0 \
-global PIIX4_PM.disable_s4=0 \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 \
+-usb \
-device usb-ccid,id=ua-myCCID,bus=ua-SomeWeirdController.0,port=1 \
-device usb-ccid,id=ua-myCCID2,bus=ua-SomeWeirdController.0,port=2 \
--usb \
-drive file=/var/lib/libvirt/images/fd.img,format=raw,if=none,\
id=drive-ua-myDisk1,cache=none \
-drive file=/var/lib/libvirt/images/gentoo.qcow2,format=qcow2,if=none,\
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 09/11] qemuBuildControllersByTypeCommandLine: free devstr in the cleanup section
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7aafe235e2..5c7e53bb6a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3117,12 +3117,12 @@ qemuBuildControllersByTypeCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
virDomainControllerType type)
{
+ char *devstr = NULL;
int ret = -1;
size_t i;
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
- char *devstr;
if (cont->type != type)
continue;
@@ -3155,23 +3155,22 @@ qemuBuildControllersByTypeCommandLine(virCommandPtr cmd,
continue;
}
+ VIR_FREE(devstr);
if (qemuBuildControllerDevStr(def, cont, qemuCaps, &devstr) < 0)
goto cleanup;
if (devstr) {
- if (qemuCommandAddExtDevice(cmd, &cont->info) < 0) {
- VIR_FREE(devstr);
+ if (qemuCommandAddExtDevice(cmd, &cont->info) < 0)
goto cleanup;
- }
virCommandAddArg(cmd, "-device");
virCommandAddArg(cmd, devstr);
- VIR_FREE(devstr);
}
}
ret = 0;
cleanup:
+ VIR_FREE(devstr);
return ret;
}
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 10/11] rename qemuBuildControllerDevCommandLine
Use qemuBuildControllersCommandLine since it builds the command line
for (nearly) all controllers, not just one.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5c7e53bb6a..214d4068e9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3176,9 +3176,9 @@ qemuBuildControllersByTypeCommandLine(virCommandPtr cmd,
static int
-qemuBuildControllerDevCommandLine(virCommandPtr cmd,
- const virDomainDef *def,
- virQEMUCapsPtr qemuCaps)
+qemuBuildControllersCommandLine(virCommandPtr cmd,
+ const virDomainDef *def,
+ virQEMUCapsPtr qemuCaps)
{
size_t j;
int contOrder[] = {
@@ -10597,7 +10597,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildControllerDevCommandLine(cmd, def, qemuCaps) < 0)
+ if (qemuBuildControllersCommandLine(cmd, def, qemuCaps) < 0)
goto error;
if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0)
--
2.20.1
1:56 a.m.
New subject: [libvirt] [PATCH 11/11] qemuBuildControllersCommandLine: use i instead of j as the counter
Now that the nested loop is gone.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_command.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 214d4068e9..093a528bec 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3180,7 +3180,7 @@ qemuBuildControllersCommandLine(virCommandPtr cmd,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
- size_t j;
+ size_t i;
int contOrder[] = {
/*
* List of controller types that we add commandline args for,
@@ -3209,8 +3209,8 @@ qemuBuildControllersCommandLine(virCommandPtr cmd,
};
int ret = -1;
- for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) {
- if (qemuBuildControllersByTypeCommandLine(cmd, def, qemuCaps, contOrder[j]) <
0)
+ for (i = 0; i < ARRAY_CARDINALITY(contOrder); i++) {
+ if (qemuBuildControllersByTypeCommandLine(cmd, def, qemuCaps, contOrder[i]) <
0)
goto cleanup;
}
--
2.20.1
3:25 p.m.
On 01/16/2019 02:56 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1599971
Ján Tomko (11):
qemu: move out legacy USB controller formatting
qemu: exit early if USB_CONTROLLER_MODEL_NONE is present
Add qemuBuildDomainForbidLegacyUSBController
qemu: separate counting of legacy USB controllers
qemu: separate counting of USB controllers
qemuBuildControllerDevStr: remove nusbcontroller argument
qemu: Introduce qemuBuildControllersByTypeCommandLine
qemu: format CCID controllers after USB hubs
qemuBuildControllersByTypeCommandLine: free devstr in the cleanup
section
rename qemuBuildControllerDevCommandLine
qemuBuildControllersCommandLine: use i instead of j as the counter
src/qemu/qemu_command.c | 222 +++++++++++-------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
tests/qemuxml2argvdata/chardev-reconnect.args | 2 +-
tests/qemuxml2argvdata/controller-order.args | 2 +-
tests/qemuxml2argvdata/name-escape.args | 2 +-
.../smartcard-controller.args | 2 +-
.../smartcard-host-certificates-database.args | 2 +-
.../smartcard-host-certificates.args | 2 +-
tests/qemuxml2argvdata/smartcard-host.args | 2 +-
.../smartcard-passthrough-spicevmc.args | 2 +-
.../smartcard-passthrough-tcp.args | 2 +-
tests/qemuxml2argvdata/user-aliases.args | 2 +-
13 files changed, 150 insertions(+), 97 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
Nice cleanups. Would be nice to move that 'Multiple legacy' error to
Validate time, but that can come later
Thanks,
Cole
2160
days inactive
2168
days old
13 comments
2 participants
participants (2)
-
Cole Robinson -
Ján Tomko