12:21 p.m.
Add two capabilities flags for the model of scsi controller LSI and
virtio-scsi.
Use the lsilogic model by default, if current QEMU supports doesn't it,
raise an error.
When scsi model is given in XML explicitly, we try to check if
the underlying QEMU supports it or not, raise an error on checking
failure.
Guannan Ren(3)
qemu: add capabilities flags related to scsi controller
qemu: add two qemu caps for lsi and virtio-scsi SCSI
add lsi and virtio-scsi qemu caps in testcases.
src/qemu/qemu_capabilities.c | 7 +++
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++----------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_process.c | 9 ++-
tests/qemuhelptest.c | 10 +++-
tests/qemuxml2argvtest.c | 16 ++++--
7 files changed, 114 insertions(+), 39 deletions(-)
12:21 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: add capabilities flags related to scsi controller
QEMU_CAPS_SCSI_LSI
set the flag when "lsi53c895a", bus PCI, alias "lsi" in
the output of "qemu -device ?"
-device lsi in qemu command line
QEMU_CAPS_VIRIO_SCSI_PCI
set the flag when "name "virtio-scsi-pci", bus PCI" in
the output of qemu devices query.
-device virtio-scsi-pci in qemu command line
---
src/qemu/qemu_capabilities.c | 7 +++++++
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 82a2870..ac90162 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -170,6 +170,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"balloon-event",
"bridge", /* 100 */
+ "lsi",
+ "virtio-scsi-pci",
);
@@ -1455,6 +1457,11 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags)
strstr(str, "name \"virtio-serial-s390\""))
qemuCapsSet(flags, QEMU_CAPS_VIRTIO_S390);
+ if (strstr(str, "name \"lsi53c895a\""))
+ qemuCapsSet(flags, QEMU_CAPS_SCSI_LSI);
+ if (strstr(str, "name \"virtio-scsi-pci\""))
+ qemuCapsSet(flags, QEMU_CAPS_VIRIO_SCSI_PCI);
+
/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC) &&
strstr(str, "name \"spicevmc\""))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c1b67a6..05e942e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -136,6 +136,8 @@ enum qemuCapsFlags {
QEMU_CAPS_VIRTIO_S390 = 98, /* -device virtio-*-s390 */
QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */
QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */
+ QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */
+ QEMU_CAPS_VIRIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.7.7.6
4:46 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: add capabilities flags related to scsi controller
On 08/07/2012 11:21 AM, Guannan Ren wrote:
QEMU_CAPS_SCSI_LSI
set the flag when "lsi53c895a", bus PCI, alias "lsi" in
the output of "qemu -device ?"
-device lsi in qemu command line
QEMU_CAPS_VIRIO_SCSI_PCI
set the flag when "name "virtio-scsi-pci", bus PCI" in
the output of qemu devices query.
-device virtio-scsi-pci in qemu command line
---
src/qemu/qemu_capabilities.c | 7 +++++++
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:54 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: add capabilities flags related to scsi controller
On 08/07/2012 03:46 PM, Eric Blake wrote:
On 08/07/2012 11:21 AM, Guannan Ren wrote:
> QEMU_CAPS_SCSI_LSI
> set the flag when "lsi53c895a", bus PCI, alias "lsi" in
> the output of "qemu -device ?"
> -device lsi in qemu command line
>
> QEMU_CAPS_VIRIO_SCSI_PCI
> set the flag when "name "virtio-scsi-pci", bus PCI" in
> the output of qemu devices query.
> -device virtio-scsi-pci in qemu command line
> ---
> src/qemu/qemu_capabilities.c | 7 +++++++
> src/qemu/qemu_capabilities.h | 2 ++
> 2 files changed, 9 insertions(+), 0 deletions(-)
>
ACK.
I spoke too soon. In isolation, this patch fails 'make check'. Please
take the appropriate hunks out of 3/3 and squash them into this patch so
that the testsuite passes (that way, 'git bisect' is easier to run
without hitting false failures).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:21 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model > 0) checking if the
underlying QEMU supports it or not first, raise an error on checking
failure.
When the model is not given(mode <= 0), return LSI by default, if
the QEMU doesn't support it, raise an error.
---
src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++-----------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_process.c | 9 +++-
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9999a05..d4791c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,58 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
- if (STREQ(def->os.arch, "ppc64") &&
- STREQ(def->os.machine, "pseries")) {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+ if (*model > 0) {
+ switch (*model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "lsi scsi controller"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+ 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;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported controller model: %s"),
+ virDomainControllerModelSCSITypeToString(*model));
+ return -1;
+ }
} else {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ if (STREQ(def->os.arch, "ppc64") &&
+ STREQ(def->os.machine, "pseries")) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to determine model for scsi
controller"));
+ return -1;
+ }
}
+
+ return 0;
}
/* Our custom -drive naming scheme used with id= */
static int
qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
- virDomainDiskDefPtr disk)
+ virDomainDiskDefPtr disk,
+ virBitmapPtr qemuCaps)
{
const char *prefix = virDomainDiskBusTypeToString(disk->bus);
int controllerModel = -1;
@@ -491,11 +530,10 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- }
- if (controllerModel == -1 ||
- controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
- controllerModel = qemuDefaultScsiControllerModel(def);
+ if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) <
0)
+ return -1;
+ }
if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI ||
controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
@@ -533,7 +571,7 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
{
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
- return qemuAssignDeviceDiskAliasCustom(vmdef, def);
+ return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps);
else
return qemuAssignDeviceDiskAliasFixed(def);
} else {
@@ -850,13 +888,24 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def,
virDomainDeviceInfoPtr info,
return 0;
}
-int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps)
{
int i, rc;
int model;
+ virBitmapPtr localCaps = NULL;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
+ if (!qemuCaps) {
+ /* need to get information from real environment */
+ if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch,
+ false, NULL,
+ &localCaps) < 0)
+ goto cleanup;
+ qemuCaps = localCaps;
+ }
+
for (i = 0 ; i < def->nnets; i++) {
if (def->nets[i]->model &&
STREQ(def->nets[i]->model, "spapr-vlan"))
@@ -864,21 +913,24 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
rc = qemuAssignSpaprVIOAddress(def, &def->nets[i]->info,
0x1000ul);
if (rc)
- return rc;
+ goto cleanup;
}
for (i = 0 ; i < def->ncontrollers; i++) {
model = def->controllers[i]->model;
- if (model == -1 &&
- def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
- model = qemuDefaultScsiControllerModel(def);
+ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+ rc = qemuCheckScsiControllerModel(def, qemuCaps, &model);
+
+ if (rc)
+ goto cleanup;
+
if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI &&
def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
def->controllers[i]->info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info,
0x2000ul);
if (rc)
- return rc;
+ goto cleanup;
}
for (i = 0 ; i < def->nserials; i++) {
@@ -890,12 +942,16 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
rc = qemuAssignSpaprVIOAddress(def, &def->serials[i]->info,
0x30000000ul);
if (rc)
- return rc;
+ goto cleanup;
}
/* No other devices are currently supported on spapr-vio */
return 0;
+
+cleanup:
+ qemuCapsFree(localCaps);
+ return -1;
}
#define QEMU_PCI_ADDRESS_LAST_SLOT 31
@@ -1059,7 +1115,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
{
int rc;
- rc = qemuDomainAssignSpaprVIOAddresses(def);
+ rc = qemuDomainAssignSpaprVIOAddresses(def, qemuCaps);
if (rc)
return rc;
@@ -2433,9 +2489,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if (controllerModel == -1 ||
- controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
- controllerModel = qemuDefaultScsiControllerModel(def);
+ if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0)
+ goto error;
if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
if (disk->info.addr.drive.target != 0) {
@@ -2764,10 +2819,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
model = def->model;
- if (model == -1 ||
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) {
- model = qemuDefaultScsiControllerModel(domainDef);
- }
+ if ((qemuCheckScsiControllerModel(domainDef, qemuCaps, &model)) < 0)
+ return NULL;
+
switch (model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
virBufferAddLit(&buf, "virtio-scsi-pci");
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 946a7ac..e999bc7 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -180,7 +180,8 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
int qemuDomainAssignAddresses(virDomainDefPtr def,
virBitmapPtr qemuCaps,
virDomainObjPtr);
-int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def);
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps);
int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
virBitmapPtr qemuCaps,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3a08c5b..df4a016 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3120,7 +3120,8 @@ qemuProcessReconnect(void *opaque)
}
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
- qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj);
+ if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj)) < 0)
+ goto error;
if (virSecurityManagerReserveLabel(driver->securityManager, obj->def,
obj->pid) < 0)
goto error;
@@ -3585,7 +3586,8 @@ int qemuProcessStart(virConnectPtr conn,
*/
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm);
+ if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0)
+ goto cleanup;
}
VIR_DEBUG("Building emulator command line");
@@ -4267,7 +4269,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
*/
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
VIR_DEBUG("Assigning domain PCI addresses");
- qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm);
+ if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0)
+ goto cleanup;
}
if ((timestamp = virTimeStringNow()) == NULL) {
--
1.7.7.6
4:55 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/07/2012 11:21 AM, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to
qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model > 0) checking if the
underlying QEMU supports it or not first, raise an error on checking
failure.
When the model is not given(mode <= 0), return LSI by default, if
the QEMU doesn't support it, raise an error.
---
src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++-----------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_process.c | 9 +++-
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9999a05..d4791c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,58 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr
disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
- if (STREQ(def->os.arch, "ppc64") &&
- STREQ(def->os.machine, "pseries")) {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+ if (*model > 0) {
+ switch (*model) {
This looks a bit odd; why not just:
switch (*model) {
...
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "lsi scsi controller"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+ 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;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported controller model: %s"),
+ virDomainControllerModelSCSITypeToString(*model));
+ return -1;
+ }
} else {
...then fold this else branch into:
case 0:
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ if (STREQ(def->os.arch, "ppc64") &&
+ STREQ(def->os.machine, "pseries")) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to determine model for scsi
controller"));
+ return -1;
+ }
}
+
+ return 0;
}
But that's just a formatting recommendation, and not a bug in your code,
so I'm okay if you leave it as written.
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps)
{
int i, rc;
int model;
+ virBitmapPtr localCaps = NULL;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
+ if (!qemuCaps) {
+ /* need to get information from real environment */
+ if (qemuCapsExtractVersionInfo(def->emulator, def->os.arch,
+ false, NULL,
+ &localCaps) < 0)
+ goto cleanup;
+ qemuCaps = localCaps;
+ }
Yet more evidence why we need to rewrite the capabilities collection
code to be caching, but that's a project for another day.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:03 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/07/2012 03:55 PM, Eric Blake wrote:
On 08/07/2012 11:21 AM, Guannan Ren wrote:
> Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
> When scsi model is given explicitly in XML(model > 0) checking if the
> underlying QEMU supports it or not first, raise an error on checking
> failure.
> When the model is not given(mode <= 0), return LSI by default, if
> the QEMU doesn't support it, raise an error.
ACK.
This patch introduces even more testsuite breaks than 1/3. Please fold
the rest of 3/3 into this patch, and when you push, you should have just
two patches, both with testsuite improvements, and where both patches in
isolation pass 'make check'.
And don't forget the /VIRIO/VIRTIO/ spelling fix.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:23 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/08/2012 07:03 AM, Eric Blake wrote:
On 08/07/2012 03:55 PM, Eric Blake wrote:
> On 08/07/2012 11:21 AM, Guannan Ren wrote:
>> Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
>> When scsi model is given explicitly in XML(model > 0) checking if the
>> underlying QEMU supports it or not first, raise an error on checking
>> failure.
>> When the model is not given(mode <= 0), return LSI by default, if
>> the QEMU doesn't support it, raise an error.
> ACK.
This patch introduces even more testsuite breaks than 1/3. Please fold
the rest of 3/3 into this patch, and when you push, you should have just
two patches, both with testsuite improvements, and where both patches in
isolation pass 'make check'.
And don't forget the /VIRIO/VIRTIO/ spelling fix.
Thanks for the review.
Finally, it comes to two patches with separate fixed testcase
squashed in
, each of them could pass 'make check'.
The spelling is fixed too.
I choose keeping the format of switch statement code. The *model
could be
value 0 (VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
or value -1
model = virXMLPropString(node, "model");
if (!model)
def->model = -1;
Pushed.
Guannan
5:28 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On Wed, Aug 08, 2012 at 01:21:10AM +0800, Guannan Ren wrote:
Rename qemuDefaultScsiControllerModel to
qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model > 0) checking if the
underlying QEMU supports it or not first, raise an error on checking
failure.
When the model is not given(mode <= 0), return LSI by default, if
the QEMU doesn't support it, raise an error.
---
src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++-----------
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_process.c | 9 +++-
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9999a05..d4791c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,58 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr
disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
- if (STREQ(def->os.arch, "ppc64") &&
- STREQ(def->os.machine, "pseries")) {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
This method is *modifying* the value in the 'model' parameter, so
using 'Check' is a horribly misleading name. This should be 'Set'
or something similar to make it clear that it is modifying the
parameters, not merely checking them..
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:11 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
On 08/08/2012 06:28 PM, Daniel P. Berrange wrote:
On Wed, Aug 08, 2012 at 01:21:10AM +0800, Guannan Ren wrote:
> Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
> When scsi model is given explicitly in XML(model > 0) checking if the
> underlying QEMU supports it or not first, raise an error on checking
> failure.
> When the model is not given(mode <= 0), return LSI by default, if
> the QEMU doesn't support it, raise an error.
> ---
> src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++++++++++-----------
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_process.c | 9 +++-
> 3 files changed, 88 insertions(+), 30 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9999a05..d4791c6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -469,19 +469,58 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr
disk)
> }
>
> static int
> -qemuDefaultScsiControllerModel(virDomainDefPtr def) {
> - if (STREQ(def->os.arch, "ppc64") &&
> - STREQ(def->os.machine, "pseries")) {
> - return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
> +qemuCheckScsiControllerModel(virDomainDefPtr def,
> + virBitmapPtr qemuCaps,
> + int *model)
This method is *modifying* the value in the 'model' parameter, so
using 'Check' is a horribly misleading name. This should be 'Set'
or something similar to make it clear that it is modifying the
parameters, not merely checking them..
Daniel
Agreed, I will post a patch to change that.
Thanks.
9:46 a.m.
New subject: [libvirt] [PATCH] qemu:rename qemuCheckScsiControllerModel function
From qemuCheckScsiControllerModel
To
qemuSetScsiControllerModel
---
src/qemu/qemu_command.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 56943d5..eb3ef26 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuCheckScsiControllerModel(virDomainDefPtr def,
+qemuSetScsiControllerModel(virDomainDefPtr def,
virBitmapPtr qemuCaps,
int *model)
{
@@ -531,7 +531,7 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) <
0)
+ if ((qemuSetScsiControllerModel(def, qemuCaps, &controllerModel)) <
0)
return -1;
}
@@ -919,7 +919,7 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
for (i = 0 ; i < def->ncontrollers; i++) {
model = def->controllers[i]->model;
if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
- rc = qemuCheckScsiControllerModel(def, qemuCaps, &model);
+ rc = qemuSetScsiControllerModel(def, qemuCaps, &model);
if (rc)
goto cleanup;
}
@@ -2489,7 +2489,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0)
+ if ((qemuSetScsiControllerModel(def, qemuCaps, &controllerModel)) < 0)
goto error;
if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
@@ -2819,7 +2819,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
model = def->model;
- if ((qemuCheckScsiControllerModel(domainDef, qemuCaps, &model)) < 0)
+ if ((qemuSetScsiControllerModel(domainDef, qemuCaps, &model)) < 0)
return NULL;
switch (model) {
--
1.7.7.6
9:54 a.m.
New subject: [libvirt] [PATCH] qemu:rename qemuCheckScsiControllerModel function
On 08/08/2012 08:46 AM, Guannan Ren wrote:
>From qemuCheckScsiControllerModel
To qemuSetScsiControllerModel
---
src/qemu/qemu_command.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 56943d5..eb3ef26 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuCheckScsiControllerModel(virDomainDefPtr def,
+qemuSetScsiControllerModel(virDomainDefPtr def,
virBitmapPtr qemuCaps,
int *model)
Fix the indentation on the next two lines.
ACK with that tweak.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:11 a.m.
New subject: [libvirt] [PATCH] qemu:rename qemuCheckScsiControllerModel function
On 08/08/2012 10:54 PM, Eric Blake wrote:
On 08/08/2012 08:46 AM, Guannan Ren wrote:
> >From qemuCheckScsiControllerModel
> To qemuSetScsiControllerModel
> ---
> src/qemu/qemu_command.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 56943d5..eb3ef26 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -469,7 +469,7 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr
disk)
> }
>
> static int
> -qemuCheckScsiControllerModel(virDomainDefPtr def,
> +qemuSetScsiControllerModel(virDomainDefPtr def,
> virBitmapPtr qemuCaps,
> int *model)
Fix the indentation on the next two lines.
ACK with that tweak.
Fixed.
Thanks for the review.
Pushed.
Guannan
12:21 p.m.
New subject: [libvirt] [PATCH 3/3] test: add lsi and virtio-scsi qemu caps in testcases.
---
tests/qemuhelptest.c | 10 +++++++---
tests/qemuxml2argvtest.c | 16 ++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 75c818c..28d742e 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -482,7 +482,8 @@ mymain(void)
QEMU_CAPS_PCI_ROMBAR,
QEMU_CAPS_NO_ACPI,
QEMU_CAPS_VIRTIO_BLK_SG_IO,
- QEMU_CAPS_CPU_HOST);
+ QEMU_CAPS_CPU_HOST,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -677,7 +678,8 @@ mymain(void)
QEMU_CAPS_FSDEV_WRITEOUT,
QEMU_CAPS_SCSI_BLOCK,
QEMU_CAPS_SCSI_CD,
- QEMU_CAPS_IDE_CD);
+ QEMU_CAPS_IDE_CD,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("qemu-1.1.0", 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -755,7 +757,9 @@ mymain(void)
QEMU_CAPS_NO_USER_CONFIG,
QEMU_CAPS_HDA_MICRO,
QEMU_CAPS_NEC_USB_XHCI,
- QEMU_CAPS_NETDEV_BRIDGE);
+ QEMU_CAPS_NETDEV_BRIDGE,
+ QEMU_CAPS_SCSI_LSI,
+ QEMU_CAPS_VIRIO_SCSI_PCI);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1adba78..2094984 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -458,16 +458,19 @@ mymain(void)
DO_TEST("disk-usb-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("disk-scsi-device",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("disk-scsi-device-auto",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("disk-scsi-disk-split",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
- QEMU_CAPS_SCSI_CD);
+ QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("disk-scsi-vscsi",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("disk-scsi-virtio-scsi",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("disk-sata-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE,
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI);
@@ -497,7 +500,8 @@ mymain(void)
DO_TEST("disk-scsi-lun-passthrough",
QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE,
- QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO);
+ QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO,
+ QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("graphics-vnc", NONE);
DO_TEST("graphics-vnc-socket", NONE);
@@ -757,7 +761,7 @@ mymain(void)
DO_TEST("multifunction-pci-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
- QEMU_CAPS_PCI_MULTIFUNCTION);
+ QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_SCSI_LSI);
DO_TEST("monitor-json", QEMU_CAPS_DEVICE,
QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG);
--
1.7.7.6
4:57 p.m.
New subject: [libvirt] [PATCH 3/3] test: add lsi and virtio-scsi qemu caps in testcases.
On 08/07/2012 11:21 AM, Guannan Ren wrote:
---
tests/qemuhelptest.c | 10 +++++++---
tests/qemuxml2argvtest.c | 16 ++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
@@ -755,7 +757,9 @@ mymain(void)
QEMU_CAPS_NO_USER_CONFIG,
QEMU_CAPS_HDA_MICRO,
QEMU_CAPS_NEC_USB_XHCI,
- QEMU_CAPS_NETDEV_BRIDGE);
+ QEMU_CAPS_NETDEV_BRIDGE,
+ QEMU_CAPS_SCSI_LSI,
+ QEMU_CAPS_VIRIO_SCSI_PCI);
I ack'd patch 1 too quickly. Please s/VIRIO/VIRTIO/ throughout your series.
ACK with the capability name spelling fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4489
days inactive
4490
days old
14 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Guannan Ren