[libvirt] [PATCH 0/4] Support to set disk wwn
This introduces new element <wwn> for disk, to allow to set wwn (just like setting serial number) for the virtual disk (Only QEMU devices like ide-drive, ide-hd, ide-cd, scsi-disk, scsi-hd, and scsi-cd support it). Osier Yang (4): schema: Add schema for disk <wwn> conf: Parse and format disk <wwn> qemu: Add caps to indentify if setting wwn is supported by qemu qemu: Use disk wwn in qemu command line docs/formatdomain.html.in | 5 +++ docs/schemas/basictypes.rng | 6 +++ docs/schemas/domaincommon.rng | 5 +++ docs/schemas/nodedev.rng | 6 --- src/conf/domain_conf.c | 8 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 35 ++++++++++++++++++++ .../qemuxml2argv-disk-ide-wwn.args | 6 +++ .../qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml | 28 ++++++++++++++++ .../qemuxml2argv-disk-scsi-disk-wwn.args | 10 ++++++ .../qemuxml2argv-disk-scsi-disk-wwn.xml | 35 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ 14 files changed, 152 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml -- 1.7.7.3
* docs/formatdomain.html.in: Add document. * docs/schemas/nodedev.rng: Move definition of "wwn" to ... * docs/schemas/basictypes.rng: ...Here * docs/schemas/domaincommon.rng: Add schema for disk <wwn> --- docs/formatdomain.html.in | 5 +++++ docs/schemas/basictypes.rng | 6 ++++++ docs/schemas/domaincommon.rng | 5 +++++ docs/schemas/nodedev.rng | 6 ------ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index be8489a..202aa2b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1550,6 +1550,11 @@ like <code><serial>WD-WMAP9A966149</serial></code>. <span class="since">Since 0.7.1</span> </dd> + <dt><code>wwn</code></dt> + <dd>If present, this specify WWN of virtual hard drive. It must be + composed of 16 hexadecimal digits. + <span class='since'>Since 0.10.1</span> + </dd> <dt><code>host</code></dt> <dd>The <code>host</code> element has two attributes "name" and "port", which specify the hostname and the port number. The meaning of this diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 766f9a0..38cab16 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -213,4 +213,10 @@ </data> </define> + <define name='wwn'> + <data type='string'> + <param name='pattern'>[0-9a-fA-F]{16}</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 145caf7..d1f1082 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -886,6 +886,11 @@ <optional> <ref name="geometry"/> </optional> + <optional> + <element name="wwn"> + <ref name="wwn"/> + </element> + </optional> </interleave> </define> <define name="snapshot"> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 88a4e9d..7c85815 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -245,12 +245,6 @@ </attribute> </define> - <define name='wwn'> - <data type='string'> - <param name='pattern'>[0-9a-fA-F]{16}</param> - </data> - </define> - <define name='capsfchost'> <attribute name='type'> <value>fc_host</value> -- 1.7.7.3
Il 29/08/2012 18:36, Osier Yang ha scritto:
* docs/formatdomain.html.in: Add document. * docs/schemas/nodedev.rng: Move definition of "wwn" to ... * docs/schemas/basictypes.rng: ...Here * docs/schemas/domaincommon.rng: Add schema for disk <wwn> --- docs/formatdomain.html.in | 5 +++++ docs/schemas/basictypes.rng | 6 ++++++ docs/schemas/domaincommon.rng | 5 +++++ docs/schemas/nodedev.rng | 6 ------ 4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index be8489a..202aa2b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1550,6 +1550,11 @@ like <code><serial>WD-WMAP9A966149</serial></code>. <span class="since">Since 0.7.1</span> </dd> + <dt><code>wwn</code></dt> + <dd>If present, this specify WWN of virtual hard drive. It must be
If present, this element specifies the WWN (World Wide Name) of a virtual hard disk or CD-ROM drive.
+ composed of 16 hexadecimal digits. + <span class='since'>Since 0.10.1</span> + </dd> <dt><code>host</code></dt> <dd>The <code>host</code> element has two attributes "name" and "port", which specify the hostname and the port number. The meaning of this diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 766f9a0..38cab16 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -213,4 +213,10 @@ </data> </define>
+ <define name='wwn'> + <data type='string'> + <param name='pattern'>[0-9a-fA-F]{16}</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 145caf7..d1f1082 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -886,6 +886,11 @@ <optional> <ref name="geometry"/> </optional> + <optional> + <element name="wwn"> + <ref name="wwn"/> + </element> + </optional> </interleave> </define> <define name="snapshot"> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 88a4e9d..7c85815 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -245,12 +245,6 @@ </attribute> </define>
- <define name='wwn'> - <data type='string'> - <param name='pattern'>[0-9a-fA-F]{16}</param> - </data> - </define> - <define name='capsfchost'> <attribute name='type'> <value>fc_host</value>
Looks good. Paolo
* src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk <wwn> --- src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 224aec5..1ed54a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -950,6 +950,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->mirror); VIR_FREE(def->mirrorFormat); VIR_FREE(def->auth.username); + VIR_FREE(def->wwn); if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) VIR_FREE(def->auth.secret.usage); virStorageEncryptionFree(def->encryption); @@ -3398,6 +3399,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *authUUID = NULL; char *usageType = NULL; char *tray = NULL; + char *wwn = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3700,6 +3702,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, } else if (!serial && xmlStrEqual(cur->name, BAD_CAST "serial")) { serial = (char *)xmlNodeGetContent(cur); + } else if (!wwn && + xmlStrEqual(cur->name, BAD_CAST "wwn")) { + wwn = (char *)xmlNodeGetContent(cur); } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ } @@ -3996,6 +4001,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, encryption = NULL; def->serial = serial; serial = NULL; + def->wwn = wwn; + wwn = NULL; if (!def->driverType && caps->defaultDiskDriverType && @@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn); if (def->encryption) { virBufferAdjustIndent(buf, 6); if (virStorageEncryptionFormat(buf, def->encryption) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9ee57e1..9a05f4e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -571,6 +571,7 @@ struct _virDomainDiskDef { virDomainBlockIoTuneInfo blkdeviotune; char *serial; + char *wwn; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ -- 1.7.7.3
Il 29/08/2012 18:36, Osier Yang ha scritto:
@@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn);
What happens if the wwn was not passed? If it is NULL, it's a bug (same for serial). If it is empty, the output will not obey the schema, so the whole element should be left out. I think it's best to do the same for serial, but technically it should be ok. Paolo
Il 30/08/2012 18:14, Paolo Bonzini ha scritto:
@@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn);
What happens if the wwn was not passed?
If it is NULL, it's a bug (same for serial).
If it is empty, the output will not obey the schema, so the whole element should be left out. I think it's best to do the same for serial, but technically it should be ok.
Also, should libvirt validate the format of the WWN? Paolo
On 2012年08月31日 00:17, Paolo Bonzini wrote:
Il 30/08/2012 18:14, Paolo Bonzini ha scritto:
@@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->transient) virBufferAddLit(buf, "<transient/>\n"); virBufferEscapeString(buf, "<serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn);
What happens if the wwn was not passed?
If it is NULL, it's a bug (same for serial).
If it is empty, the output will not obey the schema, so the whole element should be left out. I think it's best to do the same for serial, but technically it should be ok.
Also, should libvirt validate the format of the WWN?
Oh, sure, v2 will have it.
On 08/30/2012 09:14 AM, Paolo Bonzini wrote:
Il 29/08/2012 18:36, Osier Yang ha scritto:
@@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn);
What happens if the wwn was not passed?
virBufferEscapeString() is magic - if def->wwn is NULL, then the entire output string is omitted from buf. We use that idiom all over the place.
If it is NULL, it's a bug (same for serial).
If it is empty, the output will not obey the schema, so the whole element should be left out.
Yep, that is what will happen, with the code as-is. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年08月31日 00:14, Paolo Bonzini wrote:
Il 29/08/2012 18:36, Osier Yang ha scritto:
@@ -11557,6 +11564,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, "<transient/>\n"); virBufferEscapeString(buf, "<serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn);
What happens if the wwn was not passed?
If it is NULL, it's a bug (same for serial).
If it is empty, the output will not obey the schema, so the whole element should be left out. I think it's best to do the same for serial, but technically it should be ok.
See Eric's follow up explaination. I was also confused about it before not looking into virBufferEscapeString one step further though. :-) Osier
Validates the wwn while parsing, error out if it's malformed. * src/util/util.h: Declare virValidateWWN * src/util/util.c: Implement virValidateWWN * src/libvirt_private.syms: Export virValidateWWN. * src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk <wwn> --- src/conf/domain_conf.c | 12 +++++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/util/util.c | 24 ++++++++++++++++++++++++ src/util/util.h | 2 ++ 5 files changed, 39 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 554298d..df6599a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -950,6 +950,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->mirror); VIR_FREE(def->mirrorFormat); VIR_FREE(def->auth.username); + VIR_FREE(def->wwn); if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) VIR_FREE(def->auth.secret.usage); virStorageEncryptionFree(def->encryption); @@ -3353,7 +3354,6 @@ cleanup: goto cleanup; } - /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -3402,6 +3402,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *authUUID = NULL; char *usageType = NULL; char *tray = NULL; + char *wwn = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3704,6 +3705,12 @@ virDomainDiskDefParseXML(virCapsPtr caps, } else if (!serial && xmlStrEqual(cur->name, BAD_CAST "serial")) { serial = (char *)xmlNodeGetContent(cur); + } else if (!wwn && + xmlStrEqual(cur->name, BAD_CAST "wwn")) { + wwn = (char *)xmlNodeGetContent(cur); + + if (!virValidateWWN(wwn)) + goto error; } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ } @@ -4000,6 +4007,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, encryption = NULL; def->serial = serial; serial = NULL; + def->wwn = wwn; + wwn = NULL; if (!def->driverType && caps->defaultDiskDriverType && @@ -11561,6 +11570,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn); if (def->encryption) { virBufferAdjustIndent(buf, 6); if (virStorageEncryptionFormat(buf, def->encryption) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index dfdae49..3b335a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -571,6 +571,7 @@ struct _virDomainDiskDef { virDomainBlockIoTuneInfo blkdeviotune; char *serial; + char *wwn; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 27eb43e..a0675f4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1240,6 +1240,7 @@ virStrToLong_ull; virStrcpy; virStrncpy; virTrimSpaces; +virValidateWWN; virVasprintf; diff --git a/src/util/util.c b/src/util/util.c index 91eab72..682a504 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -45,6 +45,7 @@ #include <termios.h> #include <pty.h> #include <locale.h> +#include <regex.h> #if HAVE_LIBDEVMAPPER_H # include <libdevmapper.h> @@ -3052,3 +3053,26 @@ bool virIsDevMapperDevice(const char *dev_name ATTRIBUTE_UNUSED) return false; } #endif + +#define WWN_REG_PATTERN "[0-9a-zA-Z]{16}" +bool +virValidateWWN(const char *wwn) { + regex_t re; + int err; + char error[100]; + + if ((err = regcomp(&re, WWN_REG_PATTERN, REG_EXTENDED)) != 0) + goto fail; + + if ((err = regexec(&re, wwn, 0, NULL, 0)) != 0) + goto fail; + + regfree(&re); + return true; + +fail: + regerror(err, &re, error, sizeof(error)); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed wwn: %s"), error); + return false; +} diff --git a/src/util/util.h b/src/util/util.h index a5d892d..0c0efad 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -277,4 +277,6 @@ int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL; bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); +bool virValidateWWN(const char *wwn); + #endif /* __VIR_UTIL_H__ */ -- 1.7.7.3
On 2012年08月31日 19:10, Osier Yang wrote:
Validates the wwn while parsing, error out if it's malformed.
* src/util/util.h: Declare virValidateWWN * src/util/util.c: Implement virValidateWWN * src/libvirt_private.syms: Export virValidateWWN. * src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk<wwn> ---
I made a memory leak in this patch, will squash the follow diff in if pushing. diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index df6599a..40353a4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4063,6 +4063,7 @@ cleanup: VIR_FREE(copy_on_read); VIR_FREE(devaddr); VIR_FREE(serial); + VIR_FREE(wwn); virStorageEncryptionFree(encryption); VIR_FREE(startupPolicy);
On 08/31/2012 04:10 AM, Osier Yang wrote:
Validates the wwn while parsing, error out if it's malformed.
* src/util/util.h: Declare virValidateWWN * src/util/util.c: Implement virValidateWWN * src/libvirt_private.syms: Export virValidateWWN. * src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk <wwn>
+#define WWN_REG_PATTERN "[0-9a-zA-Z]{16}" +bool +virValidateWWN(const char *wwn) { + regex_t re; + int err; + char error[100]; + + if ((err = regcomp(&re, WWN_REG_PATTERN, REG_EXTENDED)) != 0)
Do we really need regcomp() for this? I'm thinking it's much faster to just do something like: for (i = 0; wwn[i]; i++) if (!c_isxdigit(wwn[i])) break; if (i != 16 || wwn[i]) // error, return false; return true; -- -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年08月31日 22:37, Eric Blake wrote:
On 08/31/2012 04:10 AM, Osier Yang wrote:
Validates the wwn while parsing, error out if it's malformed.
* src/util/util.h: Declare virValidateWWN * src/util/util.c: Implement virValidateWWN * src/libvirt_private.syms: Export virValidateWWN. * src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk<wwn>
+#define WWN_REG_PATTERN "[0-9a-zA-Z]{16}" +bool +virValidateWWN(const char *wwn) { + regex_t re; + int err; + char error[100]; + + if ((err = regcomp(&re, WWN_REG_PATTERN, REG_EXTENDED)) != 0)
Do we really need regcomp() for this? I'm thinking it's much faster to just do something like:
for (i = 0; wwn[i]; i++) if (!c_isxdigit(wwn[i])) break; if (i != 16 || wwn[i]) // error, return false; return true;
Which is more compat, I posted a v3, thanks. Regards, Osier
Validates the wwn while parsing, error out if it's malformed. * src/util/util.h: Declare virValidateWWN * src/util/util.c: Implement virValidateWWN * src/libvirt_private.syms: Export virValidateWWN. * src/conf/domain_conf.h: New member 'wwn' for disk def. * src/conf/domain_conf.c: Parse and format disk <wwn> --- src/conf/domain_conf.c | 13 ++++++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/util/util.c | 17 +++++++++++++++++ src/util/util.h | 2 ++ 5 files changed, 33 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 49327df..f83dfb7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -950,6 +950,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->mirror); VIR_FREE(def->mirrorFormat); VIR_FREE(def->auth.username); + VIR_FREE(def->wwn); if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) VIR_FREE(def->auth.secret.usage); virStorageEncryptionFree(def->encryption); @@ -3372,7 +3373,6 @@ cleanup: goto cleanup; } - /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -3421,6 +3421,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *authUUID = NULL; char *usageType = NULL; char *tray = NULL; + char *wwn = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -3723,6 +3724,12 @@ virDomainDiskDefParseXML(virCapsPtr caps, } else if (!serial && xmlStrEqual(cur->name, BAD_CAST "serial")) { serial = (char *)xmlNodeGetContent(cur); + } else if (!wwn && + xmlStrEqual(cur->name, BAD_CAST "wwn")) { + wwn = (char *)xmlNodeGetContent(cur); + + if (!virValidateWWN(wwn)) + goto error; } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ } @@ -4019,6 +4026,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, encryption = NULL; def->serial = serial; serial = NULL; + def->wwn = wwn; + wwn = NULL; if (!def->driverType && caps->defaultDiskDriverType && @@ -4073,6 +4082,7 @@ cleanup: VIR_FREE(copy_on_read); VIR_FREE(devaddr); VIR_FREE(serial); + VIR_FREE(wwn); virStorageEncryptionFree(encryption); VIR_FREE(startupPolicy); @@ -11586,6 +11596,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->transient) virBufferAddLit(buf, " <transient/>\n"); virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn); if (def->encryption) { virBufferAdjustIndent(buf, 6); if (virStorageEncryptionFormat(buf, def->encryption) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 034bebf..b91fa37 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -572,6 +572,7 @@ struct _virDomainDiskDef { virDomainBlockIoTuneInfo blkdeviotune; char *serial; + char *wwn; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6f14763..a8274d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1240,6 +1240,7 @@ virStrToLong_ull; virStrcpy; virStrncpy; virTrimSpaces; +virValidateWWN; virVasprintf; diff --git a/src/util/util.c b/src/util/util.c index 91eab72..8b1f0dc 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -3052,3 +3052,20 @@ bool virIsDevMapperDevice(const char *dev_name ATTRIBUTE_UNUSED) return false; } #endif + +bool +virValidateWWN(const char *wwn) { + int i; + + for (i = 0; wwn[i]; i++) + if (!c_isxdigit(wwn[i])) + break; + + if (i != 16 || wwn[i]) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed wwn: %s")); + return false; + } + + return true; +} diff --git a/src/util/util.h b/src/util/util.h index a5d892d..0c0efad 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -277,4 +277,6 @@ int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL; bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1); +bool virValidateWWN(const char *wwn); + #endif /* __VIR_UTIL_H__ */ -- 1.7.7.3
This assumes ide-drive.wwn, ide-hd.wwn, ide-cd.wwn were supported at the same time, similar for scsi-disk.wwn, scsi-hd.wwn, and scsi-cd.wwn. So only two new caps (QEMU_CAPS_IDE_DRIVE_WWN, and QEMU_CAPS_SCSI_DISK_WWN) are introduced. --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5472267..9d06232 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "bridge", /* 100 */ "lsi", "virtio-scsi-pci", + "ide-drive.wwn", + "scsi-disk.wwn", ); @@ -1493,6 +1495,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_BLK_SCSI); if (strstr(str, "scsi-disk.channel")) qemuCapsSet(flags, QEMU_CAPS_SCSI_DISK_CHANNEL); + if (strstr(str, "scsi-disk.wwn")) + qemuCapsSet(flags, QEMU_CAPS_SCSI_DISK_WWN); if (strstr(str, "scsi-block")) qemuCapsSet(flags, QEMU_CAPS_SCSI_BLOCK); if (strstr(str, "scsi-cd")) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d606890..9271dee 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -138,6 +138,8 @@ enum qemuCapsFlags { QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */ QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */ QEMU_CAPS_VIRTIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */ + QEMU_CAPS_IDE_DRIVE_WWN = 103, /* Is ide-drive.wwn available? */ + QEMU_CAPS_SCSI_DISK_WWN = 104, /* Is scsi-disk.wwn available? */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.7.7.3
In the subject: "identify". Otherwise looks good. Paolo Il 29/08/2012 18:36, Osier Yang ha scritto:
This assumes ide-drive.wwn, ide-hd.wwn, ide-cd.wwn were supported at the same time, similar for scsi-disk.wwn, scsi-hd.wwn, and scsi-cd.wwn. So only two new caps (QEMU_CAPS_IDE_DRIVE_WWN, and QEMU_CAPS_SCSI_DISK_WWN) are introduced. --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ 2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5472267..9d06232 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "bridge", /* 100 */ "lsi", "virtio-scsi-pci", + "ide-drive.wwn", + "scsi-disk.wwn",
);
@@ -1493,6 +1495,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_BLK_SCSI); if (strstr(str, "scsi-disk.channel")) qemuCapsSet(flags, QEMU_CAPS_SCSI_DISK_CHANNEL); + if (strstr(str, "scsi-disk.wwn")) + qemuCapsSet(flags, QEMU_CAPS_SCSI_DISK_WWN); if (strstr(str, "scsi-block")) qemuCapsSet(flags, QEMU_CAPS_SCSI_BLOCK); if (strstr(str, "scsi-cd")) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d606890..9271dee 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -138,6 +138,8 @@ enum qemuCapsFlags { QEMU_CAPS_NETDEV_BRIDGE = 100, /* bridge helper support */ QEMU_CAPS_SCSI_LSI = 101, /* -device lsi */ QEMU_CAPS_VIRTIO_SCSI_PCI = 102, /* -device virtio-scsi-pci */ + QEMU_CAPS_IDE_DRIVE_WWN = 103, /* Is ide-drive.wwn available? */ + QEMU_CAPS_SCSI_DISK_WWN = 104, /* Is scsi-disk.wwn available? */
QEMU_CAPS_LAST, /* this must always be the last item */ };
All of ide-drive, ide-hd, ide-cd, scsi-disk, scsi-hd, and scsi-cd supports wwn property. (NB, scsi-block doesn't support to set wwn). * src/qemu/qemu_command.c: Error out if underlying QEMU doesn't support wwn property for the device; Set wwn for the device otherwise. * tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args: New test * tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml: Likewise * tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args: Likewise * tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml: Likewise * tests/qemuxml2argvtest.c: Add the new tests. --- src/qemu/qemu_command.c | 35 ++++++++++++++++++++ .../qemuxml2argv-disk-ide-wwn.args | 6 +++ .../qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml | 28 ++++++++++++++++ .../qemuxml2argv-disk-scsi-disk-wwn.args | 10 ++++++ .../qemuxml2argv-disk-scsi-disk-wwn.xml | 35 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ 6 files changed, 121 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8c32a4d..128ffb4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2452,6 +2452,15 @@ qemuBuildDriveDevStr(virDomainDefPtr def, goto error; } + if (disk->wwn) { + if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only ide and scsi disk support wwn")); + goto error; + } + } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { /* make sure that both the bus and the qemu binary support * type='lun' (SG_IO). @@ -2484,6 +2493,14 @@ qemuBuildDriveDevStr(virDomainDefPtr def, goto error; } + if (disk->wwn && + !qemuCapsGet(qemuCaps, QEMU_CAPS_IDE_DRIVE_WWN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn for ide disk is not supported " + "by this QEMU")); + goto error; + } + if (qemuCapsGet(qemuCaps, QEMU_CAPS_IDE_CD)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, "ide-cd"); @@ -2507,6 +2524,21 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } } + if (disk->wwn) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn is not supported for lun device")); + goto error; + } + + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn for scsi disk is not supported " + "by this QEMU")); + goto error; + } + } + controllerModel = virDomainDiskFindControllerModel(def, disk, VIR_DOMAIN_CONTROLLER_TYPE_SCSI); @@ -2638,6 +2670,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if (bootindex && qemuCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) virBufferAsprintf(&opt, ",bootindex=%d", bootindex); + if (disk->wwn) + virBufferAsprintf(&opt, ",wwn=%s", disk->wwn); + if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args new file mode 100644 index 0000000..4b8a543 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-1,serial=WD-WMAP9A966149 \ +-device ide-hd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,wwn=5000c50015ea71ad \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml new file mode 100644 index 0000000..dccec95 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-ide-wwn.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <serial>WD-WMAP9A966149</serial> + <wwn>5000c50015ea71ad</wwn> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args new file mode 100644 index 0000000..fe4591b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-device lsi,id=scsi1,bus=pci.0,addr=0x4 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-1-0 \ +-device scsi-cd,bus=scsi0.0,channel=0,scsi-id=1,lun=0,drive=drive-scsi0-0-1-0,id=scsi0-0-1-0,wwn=5000c50015ea71ac \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-scsi0-0-0-0 \ +-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,wwn=5000c50015ea71ad \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml new file mode 100644 index 0000000..dc35548 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='cdrom'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='1' unit='0'/> + <serial>WD-WMAP9A966149</serial> + <wwn>5000c50015ea71ac</wwn> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='sdb' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <wwn>5000c50015ea71ad</wwn> + </disk> + <controller type='usb' index='0'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='scsi' index='1' model='lsilogic'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 71513fb..d5ecb31 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -466,6 +466,10 @@ mymain(void) DO_TEST("disk-scsi-disk-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI); + DO_TEST("disk-scsi-disk-wwn", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI, + QEMU_CAPS_SCSI_DISK_WWN); DO_TEST("disk-scsi-vscsi", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-virtio-scsi", @@ -787,6 +791,9 @@ mymain(void) DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); + DO_TEST("disk-ide-wwn", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_IDE_CD, + QEMU_CAPS_DRIVE_SERIAL, QEMU_CAPS_IDE_DRIVE_WWN); DO_TEST("disk-geometry", QEMU_CAPS_DRIVE); -- 1.7.7.3
Il 29/08/2012 18:36, Osier Yang ha scritto:
All of ide-drive, ide-hd, ide-cd, scsi-disk, scsi-hd, and scsi-cd supports wwn property. (NB, scsi-block doesn't support to set wwn).
Please make it an error to specify a wwn together with scsi-block. Paolo
On 2012年08月31日 00:17, Paolo Bonzini wrote:
Il 29/08/2012 18:36, Osier Yang ha scritto:
All of ide-drive, ide-hd, ide-cd, scsi-disk, scsi-hd, and scsi-cd supports wwn property. (NB, scsi-block doesn't support to set wwn).
Please make it an error to specify a wwn together with scsi-block.
Paolo
I checked it by: + if (disk->wwn) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting wwn is not supported for lun device")); + goto error; + } No words "scsi-block" though, that's why I guess you missed it. :-) Regards, Osier
participants (3)
-
Eric Blake -
Osier Yang -
Paolo Bonzini