On 2013年03月04日 14:01, Han Cheng wrote:
Adding scsi hostdev, it should like:
<hostdev mode='subsystem' type='scsi'>
<source>
<adapter name='scsi_host0'/>
<address bus='0' target='0' unit='0'/>
</source>
<address type='drive' controller='0' bus='0'
target='4' unit='8'/>
</hostdev>
---
docs/schemas/domaincommon.rng | 33 +++++++++
src/conf/domain_audit.c | 10 +++
src/conf/domain_conf.c | 149 +++++++++++++++++++++++++++++++++++++++--
src/conf/domain_conf.h | 7 ++
4 files changed, 194 insertions(+), 5 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index fbb4001..ebd0e42 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2931,6 +2931,7 @@
<choice>
<ref name="hostdevsubsyspci"/>
<ref name="hostdevsubsysusb"/>
+<ref name="hostdevsubsysscsi"/>
</choice>
</define>
@@ -2983,6 +2984,22 @@
</element>
</define>
+<define name="hostdevsubsysscsi">
+<attribute name="type">
+<value>scsi</value>
+</attribute>
+<element name="source">
+<element name="adapter">
+<attribute name="name">
+<ref name="scsiAdapter"/>
+</attribute>
+</element>
+<element name="address">
+<ref name="scsiaddress"/>
+</element>
+</element>
+</define>
+
<define name="hostdevcapsstorage">
<attribute name="type">
<value>storage</value>
@@ -3027,6 +3044,17 @@
</attribute>
</element>
</define>
+<define name="scsiaddress">
+<attribute name="bus">
+<ref name="driveBus"/>
+</attribute>
+<attribute name="target">
+<ref name="driveTarget"/>
+</attribute>
+<attribute name="unit">
+<ref name="driveUnit"/>
+</attribute>
+</define>
<define name="usbportaddress">
<attribute name="bus">
<ref name="usbAddr"/>
@@ -3893,4 +3921,9 @@
</element>
<empty/>
</define>
+<define name="scsiAdapter">
+<data type="string">
+<param name="pattern">scsi_host[0-9]{1,2}</param>
No need to have a duplicate definition. It can reuse what
storage pool uses.
+</data>
+</define>
</grammar>
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index c00bd11..e6d44b1 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -281,6 +281,16 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr
hostdev,
goto cleanup;
}
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+ if (virAsprintf(&address, "%.3d:%.3d:%.3d:%.3d",
+ hostdev->source.subsys.u.scsi.adapter,
+ hostdev->source.subsys.u.scsi.bus,
+ hostdev->source.subsys.u.scsi.target,
+ hostdev->source.subsys.u.scsi.unit)< 0) {
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+ }
+ break;
default:
VIR_WARN("Unexpected hostdev type while encoding audit message:
%d",
hostdev->source.subsys.type);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5e385e4..2be9129 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -557,7 +557,8 @@ VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
"usb",
- "pci")
+ "pci",
+ "scsi")
VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
"storage",
@@ -2928,6 +2929,96 @@ virDomainParseLegacyDeviceAddress(char *devaddr,
}
static int
+virDomainHostdevSubsysScsiDefParseXML(const xmlNodePtr node,
+ virDomainHostdevDefPtr def)
+{
+ int ret = -1;
+ xmlNodePtr cur;
If you define those variables here:
char *bus, *target, *unit;
+
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE) {
+ if (xmlStrEqual(cur->name, BAD_CAST "address")) {
+ char *bus, *target, *unit;
+
+ bus=virXMLPropString(cur, "bus");
+ if (bus) {
These codes can be simplified as:
if ((bus = virXMLPropString(cur, "bus")) < 0) {
virReportError(...);
goto out;
}
if (virStrToLong_ui(bus, NULL, 0, &def->source.subsys.u.scsi.bus) < 0) {
virReportError(...);
goto out;
}
With freeing the strings in "out". [1]
+ ret = virStrToLong_ui(bus, NULL, 0,
+&def->source.subsys.u.scsi.bus);
+ VIR_FREE(bus);
+ if (ret< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse bus %s"), bus);
+ goto out;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("scsi address needs bus
id"));
Generally we uses strings like in this case:
"bus must be specified for scsi hostdev address"
+ goto out;
+ }
+ target=virXMLPropString(cur, "target");
+ if (target) {
+ ret = virStrToLong_ui(target, NULL, 0,
+&def->source.subsys.u.scsi.target);
+ VIR_FREE(target);
+ if (ret< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse target %s"), target);
+ goto out;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("scsi address needs target
id"));
+ goto out;
+ }
+ unit=virXMLPropString(cur, "unit");
+ if (unit) {
+ ret = virStrToLong_ui(unit, NULL, 0,
+&def->source.subsys.u.scsi.unit);
+ VIR_FREE(unit);
+ if (ret< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse unit %s"), unit);
+ goto out;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("scsi address needs unit
id"));
+ goto out;
+ }
+ } else if (xmlStrEqual(cur->name, BAD_CAST "adapter")) {
+ char *adapter;
+ adapter = virXMLPropString(cur, "name");
+ if (adapter&& STRSKIP(adapter, "scsi_host")) {
+ ret = virStrToLong_ui(adapter + strlen("scsi_host"), NULL,
0,
+&def->source.subsys.u.scsi.adapter);
Why not storing the adapter name as a string instead?
+ VIR_FREE(adapter);
+ if (ret< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse adapter %s"),
adapter);
+ goto out;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("scsi needs adapter"));
+ goto out;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown scsi source type '%s'"),
+ cur->name);
+ goto out;
+ }
+ }
+ cur = cur->next;
+ }
+
How about "adapter" is specified, but "source address" is not
specified. Or the vice versa.
+ ret = 0;
+out:
[1]
VIR_FREE(bus);
VIR_FREE(target);
VIR_FREE(unit);
+ return ret;
+}
+
+static int
virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
virDomainHostdevDefPtr def)
{
@@ -3222,6 +3313,10 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
if (virDomainHostdevSubsysUsbDefParseXML(sourcenode, def)< 0)
goto error;
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+ if (virDomainHostdevSubsysScsiDefParseXML(sourcenode, def)< 0)
+ goto error;
+ break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("address type='%s' not supported in hostdev
interfaces"),
@@ -12146,6 +12241,30 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def)
return 0;
}
+static int
+virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
+{
+ /* Look for any hostdev scsi dev */
+ int i;
+ int maxController = -1;
+ virDomainHostdevDefPtr hostdev;
+
+ for (i = 0; i< def->nhostdevs; i++) {
+ hostdev = *(def->hostdevs + i);
+ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS&&
+ hostdev->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI&&
+ (int)hostdev->info->addr.drive.controller> maxController) {
+ maxController = hostdev->info->addr.drive.controller;
+ }
+ }
+
+ for (i = 0; i<= maxController; i++) {
+ if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, i)<
0)
+ return -1;
+ }
+
+ return 0;
+}
/*
* Based on the declared<address/> info for any devices,
@@ -12182,6 +12301,9 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
if (virDomainDefMaybeAddSmartcardController(def)< 0)
return -1;
+ if (virDomainDefMaybeAddHostdevSCSIcontroller(def)< 0)
+ return -1;
+
return 0;
}
@@ -12997,6 +13119,15 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virBufferAdjustIndent(buf, 2);
switch (def->source.subsys.type)
{
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+ virBufferAsprintf(buf, "<adapter
name='scsi_host%d'/>\n",
This is hard code. Assuming that a scsi host device can have different
name with "scsi_host" on platform other than Linux. So again, IMHO
we should just storing the adapter name as string.
+ def->source.subsys.u.scsi.adapter);
+ virBufferAsprintf(buf, "<address %sbus='%d' target='%d'
unit='%d'/>\n",
+ includeTypeInAddr ? "type='scsi' " :
"",
+ def->source.subsys.u.scsi.bus,
+ def->source.subsys.u.scsi.target,
+ def->source.subsys.u.scsi.unit);
+ break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
if (def->source.subsys.u.usb.vendor) {
virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n",
@@ -14250,10 +14381,18 @@ virDomainHostdevDefFormat(virBufferPtr buf,
}
virBufferAdjustIndent(buf, -6);
- if (virDomainDeviceInfoFormat(buf, def->info,
- flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT
- | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)< 0)
- return -1;
+ if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS&&
+ def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+ if (virDomainDeviceInfoFormat(buf, def->info,
+ flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT
+ | VIR_DOMAIN_XML_INTERNAL_ALLOW_READONLY)< 0)
As commented in 1/5. This is no need as long as you add the "readonly"
as an external XML tag.
+ return -1;
+ } else {
+ if (virDomainDeviceInfoFormat(buf, def->info,
+ flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT
+ | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM)< 0)
+ return -1;
+ }
virBufferAddLit(buf, "</hostdev>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 39c5849..c04cb23 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -366,6 +366,7 @@ enum virDomainHostdevMode {
enum virDomainHostdevSubsysType {
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB,
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI,
+ VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
};
@@ -385,6 +386,12 @@ struct _virDomainHostdevSubsys {
unsigned vendor;
unsigned product;
} usb;
+ struct {
+ unsigned adapter;
char *adapter;
+ unsigned bus;
+ unsigned target;
+ unsigned unit;
+ } scsi;
virDevicePCIAddress pci; /* host address */
} u;
};