5:04 p.m.
Fallout or a pile yak shavings from the series to move the controller
validation from command line building into domain xml validation:
https://www.redhat.com/archives/libvir-list/2018-January/msg00188.html
This series grabs the first patch from the other series that was
essentially already ACK'd, but instead of heading down the path of
"working around" the fact that SCSI controller model may not be
set after domain post processing, this series builds up a series
of changes to implement altering the SCSI controller model during
device post processing rather than waiting for the validation phase
to "cheat" and alter a local model value temporarily.
The first 4 patches are relatively straightforward and don't change
any of the outputs. Starting with patch 5, things get a bit more
interesting. Patch 6 is where the conversion to set the default
model for SCSI controllers starts... Patch 7 is where things got
a bit tricky w/r/t the implicit controller... Patches 8 and 9 just
perform cleanup from that setting.
If this is accepted - I'll go back to the other series to adjust
and repost; otherwise, we can determine whether the other series
is necessary or if we're just happy with the way things are.
John Ferlan (9):
qemu: Split qemuDomainSetSCSIControllerModel
conf: Rework and rename virDomainDeviceFindControllerModel
qemu: Introduce qemuDomainFindSCSIControllerModel
qemu: Introduce qemuDomainGetSCSIControllerModel
qemu: Fetch/save the default SCSI controller model during hotplug
qemu: Introduce qemuDomainSetSCSIControllerModel
conf: Allow configuration of implicit controller model
qemu: Reduce need to call qemuDomainGetSCSIControllerModel
qemu: Update qemuDomainFindSCSIControllerModel return
src/conf/domain_conf.c | 41 ++++--
src/conf/domain_conf.h | 6 +-
src/libvirt_private.syms | 2 +-
src/qemu/qemu_alias.c | 14 +-
src/qemu/qemu_alias.h | 3 +-
src/qemu/qemu_command.c | 88 +++++++++---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_domain.c | 8 +-
src/qemu/qemu_domain_address.c | 149 +++++++++++----------
src/qemu/qemu_domain_address.h | 11 +-
src/qemu/qemu_hotplug.c | 14 +-
src/vbox/vbox_common.c | 8 +-
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
.../qemuhotplug-base-with-scsi-controller-live.xml | 8 +-
...se-without-scsi-controller-live+disk-scsi-2.xml | 8 +-
tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +-
.../hostdev-scsi-lsi-iscsi-auth.xml | 2 +-
.../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +-
tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +-
20 files changed, 229 insertions(+), 146 deletions(-)
--
2.13.6
5:04 p.m.
New subject: [libvirt] [PATCH 1/9] qemu: Split qemuDomainSetSCSIControllerModel
Rather than one function serving two purposes, let's split out the
else condition which is checking whether the model can be used
during command line building based on the capabilities.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 65 ++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain_address.c | 76 ++++++++++++------------------------------
2 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 96bd0ad8e..c66307ee0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1888,6 +1888,68 @@ qemuCheckIOThreads(const virDomainDef *def,
}
+/**
+ * @qemuCaps: QEMU capabilities
+ * @model: SCSI model to check
+ *
+ * Using the @qemuCaps, let's ensure the provided @model can be supported
+ *
+ * Returns true if acceptible, false otherwise with error message set.
+ */
+static bool
+qemuBuildCheckSCSIControllerModel(virQEMUCapsPtr qemuCaps,
+ int model)
+{
+ switch ((virDomainControllerModelSCSI) model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "the LSI 53C895A SCSI controller"));
+ return false;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "virtio scsi controller"));
+ return false;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+ /*TODO: need checking work here if necessary */
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MPTSAS1068)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "the LSI SAS1068 (MPT Fusion) controller"));
+ return false;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MEGASAS)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "the LSI SAS1078 (MegaRAID) controller"));
+ return false;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported controller model: %s"),
+ virDomainControllerModelSCSITypeToString(model));
+ return false;
+ }
+
+ return true;
+}
+
+
char *
qemuBuildDriveDevStr(const virDomainDef *def,
virDomainDiskDefPtr disk,
@@ -2665,6 +2727,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
return -1;
+
+ if (!qemuBuildCheckSCSIControllerModel(qemuCaps, model))
+ return -1;
}
if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 600de85f8..86fe7ad11 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -40,66 +40,34 @@ VIR_LOG_INIT("qemu.qemu_domain_address");
#define VIO_ADDR_NVRAM 0x3000ul
+/**
+ * @def: Domain definition
+ * @qemuCaps: qemu capabilities
+ * @model: model to either return or adjust
+ *
+ * If the @model is already defined, return it immediately; otherwise,
+ * based on the @qemuCaps set the @model value to the default value.
+ *
+ * Returns @model on success, -1 on failure with error set.
+ */
int
qemuDomainSetSCSIControllerModel(const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
int *model)
{
- if (*model > 0) {
- switch (*model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support "
- "the LSI 53C895A SCSI controller"));
- return -1;
- }
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support "
- "virtio scsi controller"));
- return -1;
- }
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
- /*TODO: need checking work here if necessary */
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MPTSAS1068)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support "
- "the LSI SAS1068 (MPT Fusion) controller"));
- return -1;
- }
- break;
- case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_MEGASAS)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support "
- "the LSI SAS1078 (MegaRAID) controller"));
- return -1;
- }
- break;
- default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported controller model: %s"),
- virDomainControllerModelSCSITypeToString(*model));
- return -1;
- }
+ if (*model > 0)
+ return 0;
+
+ if (qemuDomainIsPSeries(def)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
} else {
- if (qemuDomainIsPSeries(def)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to determine model for scsi
controller"));
- return -1;
- }
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to determine model for scsi controller"));
+ return -1;
}
return 0;
--
2.13.6
5:04 p.m.
New subject: [libvirt] [PATCH 2/9] conf: Rework and rename virDomainDeviceFindControllerModel
As it turns out virDomainDeviceFindControllerModel was only ever
called for SCSI controllers using VIR_DOMAIN_CONTROLLER_TYPE_SCSI
as a parameter.
So rename to virDomainDeviceFindSCSIController and rather than
return a model, let's return a virDomainControllerDefPtr to let
the caller reference whatever it wants.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 24 +++++++++++-------------
src/conf/domain_conf.h | 6 +++---
src/libvirt_private.syms | 2 +-
src/qemu/qemu_alias.c | 8 +++++---
src/qemu/qemu_command.c | 13 ++++++++-----
src/vbox/vbox_common.c | 8 ++++----
6 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c9e3334f..b39667952 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7821,21 +7821,20 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED,
return ret;
}
-int
-virDomainDeviceFindControllerModel(const virDomainDef *def,
- virDomainDeviceInfoPtr info,
- int controllerType)
+
+virDomainControllerDefPtr
+virDomainDeviceFindSCSIController(const virDomainDef *def,
+ virDomainDeviceInfoPtr info)
{
- int model = -1;
size_t i;
for (i = 0; i < def->ncontrollers; i++) {
- if (def->controllers[i]->type == controllerType &&
+ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI
&&
def->controllers[i]->idx == info->addr.drive.controller)
- model = def->controllers[i]->model;
+ return def->controllers[i];
}
- return model;
+ return NULL;
}
virDomainDiskDefPtr
@@ -17804,17 +17803,16 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
hostdev = def->hostdevs[i];
if (virHostdevIsSCSIDevice(hostdev) &&
(int)hostdev->info->addr.drive.controller > maxController) {
- int model = -1;
+ virDomainControllerDefPtr cont;
maxController = hostdev->info->addr.drive.controller;
/* We may be creating a new controller because this one is full.
* So let's grab the model from it and update the model we're
* going to add as long as this one isn't undefined. The premise
* being keeping the same controller model for all SCSI hostdevs. */
- model = virDomainDeviceFindControllerModel(def, hostdev->info,
- VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if (model != -1)
- newModel = model;
+ cont = virDomainDeviceFindSCSIController(def, hostdev->info);
+ if (cont && cont->model != -1)
+ newModel = cont->model;
}
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ed8526092..21e004515 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2759,9 +2759,9 @@ int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char
*name)
ATTRIBUTE_RETURN_CHECK;
int virDomainDiskGetFormat(virDomainDiskDefPtr def);
void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format);
-int virDomainDeviceFindControllerModel(const virDomainDef *def,
- virDomainDeviceInfoPtr info,
- int controllerType);
+virDomainControllerDefPtr
+virDomainDeviceFindSCSIController(const virDomainDef *def,
+ virDomainDeviceInfoPtr info);
virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def,
int bus,
char *dst);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 792fb6056..68a1f496e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -298,7 +298,7 @@ virDomainDeviceAliasIsUserAlias;
virDomainDeviceDefCopy;
virDomainDeviceDefFree;
virDomainDeviceDefParse;
-virDomainDeviceFindControllerModel;
+virDomainDeviceFindSCSIController;
virDomainDeviceGetInfo;
virDomainDeviceInfoIterate;
virDomainDeviceTypeToString;
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 37fe2aa80..ed96f9f4c 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -190,9 +190,11 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
- controllerModel =
- virDomainDeviceFindControllerModel(def, &disk->info,
- VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
+ virDomainControllerDefPtr cont;
+
+ cont = virDomainDeviceFindSCSIController(def, &disk->info);
+ if (cont)
+ controllerModel = cont->model;
if ((qemuDomainSetSCSIControllerModel(def, qemuCaps,
&controllerModel)) < 0)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c66307ee0..113a2919b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1957,6 +1957,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
+ virDomainControllerDefPtr cont;
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
const char *contAlias;
char *drivealias;
@@ -2042,9 +2043,9 @@ qemuBuildDriveDevStr(const virDomainDef *def,
goto error;
}
- controllerModel =
- virDomainDeviceFindControllerModel(def, &disk->info,
- VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
+ cont = virDomainDeviceFindSCSIController(def, &disk->info);
+ if (cont)
+ controllerModel = cont->model;
if ((qemuDomainSetSCSIControllerModel(def, qemuCaps,
&controllerModel)) < 0)
goto error;
@@ -5142,12 +5143,14 @@ qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virDomainControllerDefPtr cont;
int model = -1;
char *driveAlias;
const char *contAlias;
- model = virDomainDeviceFindControllerModel(def, dev->info,
- VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
+ cont = virDomainDeviceFindSCSIController(def, dev->info);
+ if (cont)
+ model = cont->model;
if (qemuDomainSetSCSIControllerModel(def, qemuCaps, &model) < 0)
goto error;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 33aefabe5..6704772e6 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -1047,10 +1047,11 @@ static int
vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
{
size_t i;
- int type, ret = 0, model = -1;
+ int type, ret = 0;
const char *src = NULL;
nsresult rc = 0;
virDomainDiskDefPtr disk = NULL;
+ virDomainControllerDefPtr cont;
PRUnichar *storageCtlName = NULL;
char *controllerName = NULL;
IMedium *medium = NULL;
@@ -1126,9 +1127,8 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine
*machine)
case VIR_DOMAIN_DISK_BUS_SCSI:
VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SCSI_NAME, &storageCtlName);
- model = virDomainDeviceFindControllerModel(def, &disk->info,
- VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068) {
+ cont = virDomainDeviceFindSCSIController(def, &disk->info);
+ if (cont && cont->model ==
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068) {
VBOX_UTF16_FREE(storageCtlName);
VBOX_UTF8_TO_UTF16(VBOX_CONTROLLER_SAS_NAME, &storageCtlName);
}
--
2.13.6
5:04 p.m.
New subject: [libvirt] [PATCH 3/9] qemu: Introduce qemuDomainFindSCSIControllerModel
Rather than repeat multiple steps in order to find the SCSI
controller model, let's combine them into one helper that will
return either the model from the definition or the default
model based on the capabilities.
This patch adds an extra check/error that the controller
that's being found actually exists. This just clarifies that
the error was because the controller doesn't exist rather
than the more generic error that we were unable to determine
the model from qemuDomainSetSCSIControllerModel when a -1
was passed in and the capabilities were unable to find one.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_alias.c | 12 ++++--------
src/qemu/qemu_command.c | 17 +++++------------
src/qemu/qemu_domain_address.c | 31 +++++++++++++++++++++++++++++++
src/qemu/qemu_domain_address.h | 4 ++++
4 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index ed96f9f4c..ed3af04de 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -190,14 +190,10 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
- virDomainControllerDefPtr cont;
-
- cont = virDomainDeviceFindSCSIController(def, &disk->info);
- if (cont)
- controllerModel = cont->model;
-
- if ((qemuDomainSetSCSIControllerModel(def, qemuCaps,
- &controllerModel)) < 0)
+ controllerModel = qemuDomainFindSCSIControllerModel(def,
+ &disk->info,
+ qemuCaps);
+ if (controllerModel < 0)
return -1;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 113a2919b..32ff385cf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1957,7 +1957,6 @@ qemuBuildDriveDevStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
- virDomainControllerDefPtr cont;
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
const char *contAlias;
char *drivealias;
@@ -2043,11 +2042,9 @@ qemuBuildDriveDevStr(const virDomainDef *def,
goto error;
}
- cont = virDomainDeviceFindSCSIController(def, &disk->info);
- if (cont)
- controllerModel = cont->model;
- if ((qemuDomainSetSCSIControllerModel(def, qemuCaps,
- &controllerModel)) < 0)
+ controllerModel = qemuDomainFindSCSIControllerModel(def, &disk->info,
+ qemuCaps);
+ if (controllerModel < 0)
goto error;
if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
@@ -5143,16 +5140,12 @@ qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- virDomainControllerDefPtr cont;
int model = -1;
char *driveAlias;
const char *contAlias;
- cont = virDomainDeviceFindSCSIController(def, dev->info);
- if (cont)
- model = cont->model;
-
- if (qemuDomainSetSCSIControllerModel(def, qemuCaps, &model) < 0)
+ model = qemuDomainFindSCSIControllerModel(def, dev->info, qemuCaps);
+ if (model < 0)
goto error;
if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 86fe7ad11..74f7df205 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -74,6 +74,37 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def,
}
+/**
+ * @def: Domain definition
+ * @info: Domain device info
+ * @qemuCaps: QEMU capabilities
+ *
+ * Using the device info, find the controller related to the
+ * device by index and use that controller to return the model.
+ *
+ * Returns the model if found, -1 if not with an error message set
+ */
+int
+qemuDomainFindSCSIControllerModel(const virDomainDef *def,
+ virDomainDeviceInfoPtr info,
+ virQEMUCapsPtr qemuCaps)
+{
+ virDomainControllerDefPtr cont;
+ int model;
+
+ if (!(cont = virDomainDeviceFindSCSIController(def, info))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to find a SCSI controller for idx=%d"),
+ info->addr.drive.controller);
+ return -1;
+ }
+
+ model = cont->model;
+ ignore_value(qemuDomainSetSCSIControllerModel(def, qemuCaps, &model));
+ return model;
+}
+
+
static int
qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def)
{
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index e951a4c88..8a4346892 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -32,6 +32,10 @@ int qemuDomainSetSCSIControllerModel(const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
int *model);
+int qemuDomainFindSCSIControllerModel(const virDomainDef *def,
+ virDomainDeviceInfoPtr info,
+ virQEMUCapsPtr qemuCaps);
+
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virQEMUDriverPtr driver,
--
2.13.6
5:04 p.m.
New subject: [libvirt] [PATCH 4/9] qemu: Introduce qemuDomainGetSCSIControllerModel
Rename and rework qemuDomainSetSCSIControllerModel since we're
really not setting the SCSI controller model. Instead the code
is either returning the existing SCSI controller model value, the
default value based on the capabilities, or -1 with the error set.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 3 ++-
src/qemu/qemu_domain_address.c | 51 +++++++++++++++++++-----------------------
src/qemu/qemu_domain_address.h | 6 ++---
3 files changed, 28 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 32ff385cf..1142f302a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2723,7 +2723,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
*devstr = NULL;
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
+ model = qemuDomainGetSCSIControllerModel(domainDef, def, qemuCaps);
+ if (model < 0)
return -1;
if (!qemuBuildCheckSCSIControllerModel(qemuCaps, model))
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 74f7df205..5211b4f8a 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -42,35 +42,33 @@ VIR_LOG_INIT("qemu.qemu_domain_address");
/**
* @def: Domain definition
+ * @cont: Domain controller def
* @qemuCaps: qemu capabilities
- * @model: model to either return or adjust
*
- * If the @model is already defined, return it immediately; otherwise,
- * based on the @qemuCaps set the @model value to the default value.
+ * If the controller model is already defined, return it immediately;
+ * otherwise, based on the @qemuCaps return a default model value.
*
- * Returns @model on success, -1 on failure with error set.
+ * Returns model on success, -1 on failure with error set.
*/
int
-qemuDomainSetSCSIControllerModel(const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- int *model)
+qemuDomainGetSCSIControllerModel(const virDomainDef *def,
+ const virDomainControllerDef *cont,
+ virQEMUCapsPtr qemuCaps)
{
- if (*model > 0)
- return 0;
-
- if (qemuDomainIsPSeries(def)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to determine model for scsi controller"));
- return -1;
- }
+ if (cont->model > 0)
+ return cont->model;
- return 0;
+ if (qemuDomainIsPSeries(def))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to determine model for SCSI controller idx=%d"),
+ cont->idx);
+ return -1;
}
@@ -90,7 +88,6 @@ qemuDomainFindSCSIControllerModel(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virDomainControllerDefPtr cont;
- int model;
if (!(cont = virDomainDeviceFindSCSIController(def, info))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -99,9 +96,7 @@ qemuDomainFindSCSIControllerModel(const virDomainDef *def,
return -1;
}
- model = cont->model;
- ignore_value(qemuDomainSetSCSIControllerModel(def, qemuCaps, &model));
- return model;
+ return qemuDomainGetSCSIControllerModel(def, cont, qemuCaps);
}
@@ -227,9 +222,9 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
- model = cont->model;
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- if (qemuDomainSetSCSIControllerModel(def, qemuCaps, &model) < 0)
+ model = qemuDomainGetSCSIControllerModel(def, cont, qemuCaps);
+ if (model < 0)
goto cleanup;
}
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index 8a4346892..209b59e0d 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -28,9 +28,9 @@
# include "qemu_conf.h"
# include "qemu_capabilities.h"
-int qemuDomainSetSCSIControllerModel(const virDomainDef *def,
- virQEMUCapsPtr qemuCaps,
- int *model);
+int qemuDomainGetSCSIControllerModel(const virDomainDef *def,
+ const virDomainControllerDef *cont,
+ virQEMUCapsPtr qemuCaps);
int qemuDomainFindSCSIControllerModel(const virDomainDef *def,
virDomainDeviceInfoPtr info,
--
2.13.6
5:04 p.m.
New subject: [libvirt] [PATCH 5/9] qemu: Fetch/save the default SCSI controller model during hotplug
If we're going to add a controller to the domain, let's set the
default SCSI model value if we cannot find another SCSI controller
already present.
NB: Requires updating the live output test data since the model
will now be formatted.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 8 ++++++--
.../qemuhotplug-base-with-scsi-controller-live.xml | 8 ++++----
.../qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml | 8 ++++----
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6b245bd6a..fccbc29a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -587,6 +587,7 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
{
size_t i;
virDomainControllerDefPtr cont;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
int model = -1;
for (i = 0; i < vm->def->ncontrollers; i++) {
@@ -615,10 +616,13 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
return NULL;
cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
cont->idx = controller;
- cont->model = model;
+ if (model == -1)
+ cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont,
priv->qemuCaps);
+ else
+ cont->model = model;
VIR_INFO("No SCSI controller present, hotplugging one model=%s",
- virDomainControllerModelSCSITypeToString(model));
+ virDomainControllerModelSCSITypeToString(cont->model));
if (qemuDomainAttachControllerDevice(driver, vm, cont) < 0) {
VIR_FREE(cont);
return NULL;
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-with-scsi-controller-live.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-with-scsi-controller-live.xml
index 63eb77007..d23d3d483 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-with-scsi-controller-live.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-with-scsi-controller-live.xml
@@ -26,19 +26,19 @@
<controller type='pci' index='0' model='pci-root'>
<alias name='pci'/>
</controller>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
- <controller type='scsi' index='1'>
+ <controller type='scsi' index='1' model='virtio-scsi'>
<alias name='scsi1'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
</controller>
- <controller type='scsi' index='2'>
+ <controller type='scsi' index='2' model='virtio-scsi'>
<alias name='scsi2'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
</controller>
- <controller type='scsi' index='3'>
+ <controller type='scsi' index='3' model='virtio-scsi'>
<alias name='scsi3'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
</controller>
diff --git
a/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml
b/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml
index c12d18f71..cceb678bb 100644
---
a/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml
+++
b/tests/qemuhotplugtestdomains/qemuhotplug-base-without-scsi-controller-live+disk-scsi-2.xml
@@ -36,19 +36,19 @@
<controller type='pci' index='0' model='pci-root'>
<alias name='pci'/>
</controller>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='virtio-scsi'>
<alias name='scsi0'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
- <controller type='scsi' index='1'>
+ <controller type='scsi' index='1' model='virtio-scsi'>
<alias name='scsi1'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
</controller>
- <controller type='scsi' index='2'>
+ <controller type='scsi' index='2' model='virtio-scsi'>
<alias name='scsi2'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
</controller>
- <controller type='scsi' index='3'>
+ <controller type='scsi' index='3' model='virtio-scsi'>
<alias name='scsi3'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
</controller>
--
2.13.6
5:05 p.m.
New subject: [libvirt] [PATCH 6/9] qemu: Introduce qemuDomainSetSCSIControllerModel
During post parse processing, let's force setting the controller
model to default value if not already set for defined controllers
(e.g. the non implicit ones).
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 8 +++----
src/qemu/qemu_domain_address.c | 27 ++++++++++++++++++++++
src/qemu/qemu_domain_address.h | 4 ++++
.../hostdev-scsi-lsi-iscsi-auth.xml | 2 +-
.../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +-
tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +-
6 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b4bd3cca..c8123ce59 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4251,11 +4251,9 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
{
switch ((virDomainControllerType)cont->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- /* set the default SCSI controller model for S390 arches */
- if (cont->model == -1 &&
- ARCH_IS_S390(def->os.arch)) {
- cont->model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
- }
+ /* Set the default SCSI controller model if not already set */
+ if (qemuDomainSetSCSIControllerModel(def, cont, qemuCaps) < 0)
+ return -1;
break;
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 5211b4f8a..75c263015 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -60,6 +60,8 @@ qemuDomainGetSCSIControllerModel(const virDomainDef *def,
if (qemuDomainIsPSeries(def))
return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ else if (ARCH_IS_S390(def->os.arch))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
@@ -74,6 +76,31 @@ qemuDomainGetSCSIControllerModel(const virDomainDef *def,
/**
* @def: Domain definition
+ * @cont: Domain controller def
+ * @qemuCaps: qemu capabilities
+ *
+ * Set the controller model based on the existing value and the
+ * capabilities if possible.
+ *
+ * Returns 0 on success, -1 on failure with error set.
+ */
+int
+qemuDomainSetSCSIControllerModel(const virDomainDef *def,
+ virDomainControllerDefPtr cont,
+ virQEMUCapsPtr qemuCaps)
+{
+ int model = qemuDomainGetSCSIControllerModel(def, cont, qemuCaps);
+
+ if (model < 0)
+ return -1;
+
+ cont->model = model;
+ return 0;
+}
+
+
+/**
+ * @def: Domain definition
* @info: Domain device info
* @qemuCaps: QEMU capabilities
*
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index 209b59e0d..04fb2fc14 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -32,6 +32,10 @@ int qemuDomainGetSCSIControllerModel(const virDomainDef *def,
const virDomainControllerDef *cont,
virQEMUCapsPtr qemuCaps);
+int qemuDomainSetSCSIControllerModel(const virDomainDef *def,
+ virDomainControllerDefPtr cont,
+ virQEMUCapsPtr qemuCaps);
+
int qemuDomainFindSCSIControllerModel(const virDomainDef *def,
virDomainDeviceInfoPtr info,
virQEMUCapsPtr qemuCaps);
diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi-auth.xml
b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi-auth.xml
index cad016129..3d0efdcaf 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi-auth.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi-auth.xml
@@ -19,7 +19,7 @@
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='lsilogic'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='usb' index='0'>
diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml
b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml
index b7312ca4f..28b80a02b 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml
@@ -19,7 +19,7 @@
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='lsilogic'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='usb' index='0'>
diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml
b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml
index 54498aa5c..545ce3ba8 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml
@@ -19,7 +19,7 @@
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='lsilogic'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<controller type='usb' index='0'>
--
2.13.6
5:05 p.m.
New subject: [libvirt] [PATCH 7/9] conf: Allow configuration of implicit controller model
When an implicit controller is added, the model is defined as -1
(IOW: undefined). So, if an implicit SCSI controller was added,
can set the model to the default value if the underlying hypervisor
supports it.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 17 +++++++++++++++++
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +-
4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b39667952..8efe83489 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4924,6 +4924,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
struct virDomainDefPostParseDeviceIteratorData *data,
virHashTablePtr bootHash)
{
+ size_t i;
+
/* verify init path for container based domains */
if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -4991,6 +4993,21 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
return -1;
}
+ /* Implicit SCSI controllers without a defined model might have
+ * been added in AddImplicitDevices, after we've done the per-device
+ * post-parse. */
+ for (i = 0; i < def->ncontrollers; i++) {
+ if (def->controllers[i]->model == -1 &&
+ def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+ virDomainDeviceDef device = {
+ .type = VIR_DOMAIN_DEVICE_CONTROLLER,
+ .data.controller = def->controllers[i],
+ };
+ if (virDomainDefPostParseDeviceIterator(def, &device, NULL, data) <
0)
+ return -1;
+ }
+ }
+
/* clean up possibly duplicated metadata entries */
virXMLNodeSanitizeNamespaces(def->metadata);
diff --git a/tests/qemuargv2xmldata/nomachine-ppc64.xml
b/tests/qemuargv2xmldata/nomachine-ppc64.xml
index fa41070cb..439f9e9ac 100644
--- a/tests/qemuargv2xmldata/nomachine-ppc64.xml
+++ b/tests/qemuargv2xmldata/nomachine-ppc64.xml
@@ -33,7 +33,7 @@
<model name='spapr-pci-host-bridge'/>
<target index='0'/>
</controller>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='ibmvscsi'>
<address type='spapr-vio' reg='0x2000'/>
</controller>
<input type='keyboard' bus='usb'/>
diff --git a/tests/qemuargv2xmldata/pseries-disk.xml
b/tests/qemuargv2xmldata/pseries-disk.xml
index 601d0f7f6..1f15a950e 100644
--- a/tests/qemuargv2xmldata/pseries-disk.xml
+++ b/tests/qemuargv2xmldata/pseries-disk.xml
@@ -34,7 +34,7 @@
<model name='spapr-pci-host-bridge'/>
<target index='0'/>
</controller>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='ibmvscsi'>
<address type='spapr-vio' reg='0x2000'/>
</controller>
<input type='keyboard' bus='usb'/>
diff --git a/tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml
b/tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml
index dde374590..a0db32b56 100644
--- a/tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml
+++ b/tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml
@@ -31,7 +31,7 @@
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
</controller>
<controller type='pci' index='0' model='pci-root'/>
- <controller type='scsi' index='0'>
+ <controller type='scsi' index='0' model='lsilogic'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</controller>
<input type='mouse' bus='ps2'/>
--
2.13.6
5:05 p.m.
New subject: [libvirt] [PATCH 8/9] qemu: Reduce need to call qemuDomainGetSCSIControllerModel
Now that post parse processing handles setting the SCSI controller
model, there's no need to call qemuDomainGetSCSIControllerModel to
get the "default controller" when building the command line controller
string or when assigning the spaprvio address since the controller
model value will already be filled in.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 11 +++--------
src/qemu/qemu_domain_address.c | 14 +++-----------
2 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1142f302a..f82a78155 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2717,22 +2717,17 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
int *nusbcontroller)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int model = def->model;
const char *modelName = NULL;
*devstr = NULL;
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- model = qemuDomainGetSCSIControllerModel(domainDef, def, qemuCaps);
- if (model < 0)
- return -1;
-
- if (!qemuBuildCheckSCSIControllerModel(qemuCaps, model))
+ if (!qemuBuildCheckSCSIControllerModel(qemuCaps, def->model))
return -1;
}
if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI &&
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
+ def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) {
if (def->queues) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'queues' is only supported by virtio-scsi
controller"));
@@ -2757,7 +2752,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
switch ((virDomainControllerType) def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
- switch (model) {
+ switch (def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
virBufferAddLit(&buf, "virtio-scsi-ccw");
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 75c263015..aa736131e 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -225,12 +225,10 @@ qemuDomainAssignSpaprVIOAddress(virDomainDefPtr def,
static int
-qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps)
+qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
{
size_t i;
int ret = -1;
- int model;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
@@ -249,13 +247,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- model = qemuDomainGetSCSIControllerModel(def, cont, qemuCaps);
- if (model < 0)
- goto cleanup;
- }
-
- if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI &&
+ if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI &&
cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
cont->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
}
@@ -2864,7 +2856,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
if (qemuDomainAssignVirtioSerialAddresses(def) < 0)
return -1;
- if (qemuDomainAssignSpaprVIOAddresses(def, qemuCaps) < 0)
+ if (qemuDomainAssignSpaprVIOAddresses(def) < 0)
return -1;
if (qemuDomainAssignS390Addresses(def, qemuCaps) < 0)
--
2.13.6
5:05 p.m.
New subject: [libvirt] [PATCH 9/9] qemu: Update qemuDomainFindSCSIControllerModel return
Now that the controller model is updated during post parse callback,
this code no longer needs to fetch the model based on the capabilities
and can just return the model directly if the controller is found.
Removal of @qemuCaps cascades through various callers which are now
updated to not pass the capabilities.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_alias.c | 8 +++-----
src/qemu/qemu_alias.h | 3 +--
src/qemu/qemu_command.c | 11 ++++-------
src/qemu/qemu_command.h | 3 +--
src/qemu/qemu_domain_address.c | 6 ++----
src/qemu/qemu_domain_address.h | 3 +--
src/qemu/qemu_hotplug.c | 6 +++---
7 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index ed3af04de..e8c1256b7 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -179,8 +179,7 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
/* Our custom -drive naming scheme used with id= */
int
qemuAssignDeviceDiskAlias(virDomainDefPtr def,
- virDomainDiskDefPtr disk,
- virQEMUCapsPtr qemuCaps)
+ virDomainDiskDefPtr disk)
{
const char *prefix = virDomainDiskBusTypeToString(disk->bus);
int controllerModel = -1;
@@ -191,8 +190,7 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
controllerModel = qemuDomainFindSCSIControllerModel(def,
- &disk->info,
- qemuCaps);
+ &disk->info);
if (controllerModel < 0)
return -1;
}
@@ -539,7 +537,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
size_t i;
for (i = 0; i < def->ndisks; i++) {
- if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0)
+ if (qemuAssignDeviceDiskAlias(def, def->disks[i]) < 0)
return -1;
}
for (i = 0; i < def->nnets; i++) {
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 11bbc2860..2246e712f 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -39,8 +39,7 @@ int qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
virDomainControllerDefPtr controller);
int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
- virDomainDiskDefPtr def,
- virQEMUCapsPtr qemuCaps);
+ virDomainDiskDefPtr def);
int qemuAssignDeviceHostdevAlias(virDomainDefPtr def,
char **alias,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f82a78155..899f0cbbb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2042,8 +2042,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
goto error;
}
- controllerModel = qemuDomainFindSCSIControllerModel(def, &disk->info,
- qemuCaps);
+ controllerModel = qemuDomainFindSCSIControllerModel(def, &disk->info);
if (controllerModel < 0)
goto error;
@@ -5132,15 +5131,14 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
char *
qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
- virDomainHostdevDefPtr dev,
- virQEMUCapsPtr qemuCaps)
+ virDomainHostdevDefPtr dev)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
int model = -1;
char *driveAlias;
const char *contAlias;
- model = qemuDomainFindSCSIControllerModel(def, dev->info, qemuCaps);
+ model = qemuDomainFindSCSIControllerModel(def, dev->info);
if (model < 0)
goto error;
@@ -5623,8 +5621,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
VIR_FREE(drvstr);
virCommandAddArg(cmd, "-device");
- if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev,
- qemuCaps)))
+ if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev)))
return -1;
virCommandAddArg(cmd, devstr);
VIR_FREE(devstr);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index bdde6f918..31c9da673 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -154,8 +154,7 @@ char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);
char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
- virDomainHostdevDefPtr dev,
- virQEMUCapsPtr qemuCaps);
+ virDomainHostdevDefPtr dev);
char *
qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,
virDomainHostdevDefPtr dev,
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index aa736131e..6272b1e91 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -102,7 +102,6 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def,
/**
* @def: Domain definition
* @info: Domain device info
- * @qemuCaps: QEMU capabilities
*
* Using the device info, find the controller related to the
* device by index and use that controller to return the model.
@@ -111,8 +110,7 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def,
*/
int
qemuDomainFindSCSIControllerModel(const virDomainDef *def,
- virDomainDeviceInfoPtr info,
- virQEMUCapsPtr qemuCaps)
+ virDomainDeviceInfoPtr info)
{
virDomainControllerDefPtr cont;
@@ -123,7 +121,7 @@ qemuDomainFindSCSIControllerModel(const virDomainDef *def,
return -1;
}
- return qemuDomainGetSCSIControllerModel(def, cont, qemuCaps);
+ return cont->model;
}
diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h
index 04fb2fc14..83f8e81ca 100644
--- a/src/qemu/qemu_domain_address.h
+++ b/src/qemu/qemu_domain_address.h
@@ -37,8 +37,7 @@ int qemuDomainSetSCSIControllerModel(const virDomainDef *def,
virQEMUCapsPtr qemuCaps);
int qemuDomainFindSCSIControllerModel(const virDomainDef *def,
- virDomainDeviceInfoPtr info,
- virQEMUCapsPtr qemuCaps);
+ virDomainDeviceInfoPtr info);
int qemuDomainAssignAddresses(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index fccbc29a0..2aa927e62 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -381,7 +381,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+ if (qemuAssignDeviceDiskAlias(vm->def, disk) < 0)
goto error;
if (qemuDomainPrepareDiskSource(conn, disk, priv, cfg) < 0)
@@ -2405,7 +2405,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
if (!(drivealias = qemuAliasFromHostdev(hostdev)))
goto cleanup;
- if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
+ if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev)))
goto cleanup;
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
@@ -4634,7 +4634,7 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
}
if (!detach->info.alias) {
- if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
+ if (qemuAssignDeviceDiskAlias(vm->def, detach) < 0)
goto cleanup;
}
--
2.13.6
3:52 a.m.
On Tue, Jan 30, 2018 at 06:04:54PM -0500, John Ferlan wrote:
Fallout or a pile yak shavings from the series to move the controller
validation from command line building into domain xml validation:
https://www.redhat.com/archives/libvir-list/2018-January/msg00188.html
This series grabs the first patch from the other series that was
essentially already ACK'd, but instead of heading down the path of
"working around" the fact that SCSI controller model may not be
set after domain post processing, this series builds up a series
of changes to implement altering the SCSI controller model during
device post processing rather than waiting for the validation phase
to "cheat" and alter a local model value temporarily.
The first 4 patches are relatively straightforward and don't change
any of the outputs. Starting with patch 5, things get a bit more
interesting. Patch 6 is where the conversion to set the default
model for SCSI controllers starts... Patch 7 is where things got
a bit tricky w/r/t the implicit controller... Patches 8 and 9 just
perform cleanup from that setting.
If this is accepted - I'll go back to the other series to adjust
and repost; otherwise, we can determine whether the other series
is necessary or if we're just happy with the way things are.
John Ferlan (9):
qemu: Split qemuDomainSetSCSIControllerModel
conf: Rework and rename virDomainDeviceFindControllerModel
qemu: Introduce qemuDomainFindSCSIControllerModel
qemu: Introduce qemuDomainGetSCSIControllerModel
qemu: Fetch/save the default SCSI controller model during hotplug
qemu: Introduce qemuDomainSetSCSIControllerModel
conf: Allow configuration of implicit controller model
Could be nicer to just add an implicit controller with the correct
model.
qemu: Reduce need to call qemuDomainGetSCSIControllerModel
qemu: Update qemuDomainFindSCSIControllerModel return
src/conf/domain_conf.c | 41 ++++--
src/conf/domain_conf.h | 6 +-
src/libvirt_private.syms | 2 +-
src/qemu/qemu_alias.c | 14 +-
src/qemu/qemu_alias.h | 3 +-
src/qemu/qemu_command.c | 88 +++++++++---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_domain.c | 8 +-
src/qemu/qemu_domain_address.c | 149 +++++++++++----------
src/qemu/qemu_domain_address.h | 11 +-
src/qemu/qemu_hotplug.c | 14 +-
src/vbox/vbox_common.c | 8 +-
tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
.../qemuhotplug-base-with-scsi-controller-live.xml | 8 +-
...se-without-scsi-controller-live+disk-scsi-2.xml | 8 +-
tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +-
.../hostdev-scsi-lsi-iscsi-auth.xml | 2 +-
.../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +-
tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +-
20 files changed, 229 insertions(+), 146 deletions(-)
Either way:
ACK series
Jan
10:23 a.m.
On 01/31/2018 04:52 AM, Ján Tomko wrote:
On Tue, Jan 30, 2018 at 06:04:54PM -0500, John Ferlan wrote:
> Fallout or a pile yak shavings from the series to move the controller
> validation from command line building into domain xml validation:
>
> https://www.redhat.com/archives/libvir-list/2018-January/msg00188.html
>
> This series grabs the first patch from the other series that was
> essentially already ACK'd, but instead of heading down the path of
> "working around" the fact that SCSI controller model may not be
> set after domain post processing, this series builds up a series
> of changes to implement altering the SCSI controller model during
> device post processing rather than waiting for the validation phase
> to "cheat" and alter a local model value temporarily.
>
> The first 4 patches are relatively straightforward and don't change
> any of the outputs. Starting with patch 5, things get a bit more
> interesting. Patch 6 is where the conversion to set the default
> model for SCSI controllers starts... Patch 7 is where things got
> a bit tricky w/r/t the implicit controller... Patches 8 and 9 just
> perform cleanup from that setting.
>
> If this is accepted - I'll go back to the other series to adjust
> and repost; otherwise, we can determine whether the other series
> is necessary or if we're just happy with the way things are.
>
> John Ferlan (9):
> qemu: Split qemuDomainSetSCSIControllerModel
> conf: Rework and rename virDomainDeviceFindControllerModel
> qemu: Introduce qemuDomainFindSCSIControllerModel
> qemu: Introduce qemuDomainGetSCSIControllerModel
> qemu: Fetch/save the default SCSI controller model during hotplug
> qemu: Introduce qemuDomainSetSCSIControllerModel
> conf: Allow configuration of implicit controller model
Could be nicer to just add an implicit controller with the correct
model.
Understood; however, how do you propose to accomplish that in
domain_conf code that's supposed to be hypervisor agnostic?
It's not like we can pass around a qemuCaps so that when
virDomainDefMaybeAddController can call the get default model function.
This ended up being the least offensive way I could determine would
accomplish the task even though it.
The other thought I had was jiggling the order of the call to
virDomainDefPostParseCommon to before the iteration of the devices
(virDomainDeviceInfoIterateInternal), but doing that scared me more than
taking the option to follow how serials and videos handled similar
adjustments after post parse was called.
In any case, thanks for the speedy review. I'll push what's here and
maybe someone else will propose some other brilliant suggestion.
Tks -
John
> qemu: Reduce need to call qemuDomainGetSCSIControllerModel
> qemu: Update qemuDomainFindSCSIControllerModel return
>
> src/conf/domain_conf.c | 41 ++++--
> src/conf/domain_conf.h | 6 +-
> src/libvirt_private.syms | 2 +-
> src/qemu/qemu_alias.c | 14 +-
> src/qemu/qemu_alias.h | 3 +-
> src/qemu/qemu_command.c | 88 +++++++++---
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_domain.c | 8 +-
> src/qemu/qemu_domain_address.c | 149
> +++++++++++----------
> src/qemu/qemu_domain_address.h | 11 +-
> src/qemu/qemu_hotplug.c | 14 +-
> src/vbox/vbox_common.c | 8 +-
> tests/qemuargv2xmldata/nomachine-ppc64.xml | 2 +-
> tests/qemuargv2xmldata/pseries-disk.xml | 2 +-
> .../qemuhotplug-base-with-scsi-controller-live.xml | 8 +-
> ...se-without-scsi-controller-live+disk-scsi-2.xml | 8 +-
> tests/qemuxml2xmloutdata/disk-scsi-device-auto.xml | 2 +-
> .../hostdev-scsi-lsi-iscsi-auth.xml | 2 +-
> .../qemuxml2xmloutdata/hostdev-scsi-lsi-iscsi.xml | 2 +-
> tests/qemuxml2xmloutdata/hostdev-scsi-lsi.xml | 2 +-
> 20 files changed, 229 insertions(+), 146 deletions(-)
>
Either way:
ACK series
Jan
2483
days inactive
2484
days old
11 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko