[libvirt] [PATCH v5] npiv: Auto-generate WWN if it's not specified

The auto-generated WWN comply with the new addressing schema of WWN: <quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote> We choose hex 5 for the first nibble. And use Qumranet's OUI (00:1A:4A) as the 3-byte vendor indentifier. The last 36 bits are auto-generated. --- src/conf/node_device_conf.c | 36 +++++++++++++++++++----------------- src/libvirt_private.syms | 1 + src/util/virrandom.c | 18 ++++++++++++++++++ src/util/virrandom.h | 1 + 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index d9dc9ac..127a4ae 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -37,6 +37,7 @@ #include "buf.h" #include "uuid.h" #include "pci.h" +#include "virrandom.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -63,19 +64,12 @@ VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, static int virNodeDevCapsDefParseString(const char *xpath, xmlXPathContextPtr ctxt, - char **string, - virNodeDeviceDefPtr def, - const char *missing_error_fmt) + char **string) { char *s; - s = virXPathString(xpath, ctxt); - if (s == NULL) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - missing_error_fmt, - def->name); + if (!(s = virXPathString(xpath, ctxt))) return -1; - } *string = s; return 0; @@ -763,18 +757,26 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, if (virNodeDevCapsDefParseString("string(./wwnn[1])", ctxt, - &data->scsi_host.wwnn, - def, - _("no WWNN supplied for '%s'")) < 0) { - goto out; + &data->scsi_host.wwnn) < 0) { + if (virRandomGenerateWWN(&data->scsi_host.wwnn) < 0) { + virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, + _("no WWNN supplied for '%s', and " + "auto-generation failed"), + def->name); + goto out; + } } if (virNodeDevCapsDefParseString("string(./wwpn[1])", ctxt, - &data->scsi_host.wwpn, - def, - _("no WWPN supplied for '%s'")) < 0) { - goto out; + &data->scsi_host.wwpn) < 0) { + if (virRandomGenerateWWN(&data->scsi_host.wwpn) < 0) { + virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, + _("no WWPN supplied for '%s', and " + "auto-generation failed"), + def->name); + goto out; + } } ctxt->node = orignode2; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6ad36c..421986b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1373,6 +1373,7 @@ virPidFileDeletePath; # virrandom.h virRandomBits; +virRandomGenerateWWN; virRandomInitialize; diff --git a/src/util/virrandom.c b/src/util/virrandom.c index ec0cf03..3116275 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -22,10 +22,15 @@ #include <config.h> #include <stdlib.h> +#include <inttypes.h> #include "virrandom.h" #include "threads.h" #include "count-one-bits.h" +#include "util.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE static char randomState[128]; static struct random_data randomData; @@ -79,3 +84,16 @@ uint64_t virRandomBits(int nbits) virMutexUnlock(&randomLock); return ret; } + +#define QUMRANET_OUI "001a4a" + +int +virRandomGenerateWWN(char **wwn) { + if (virAsprintf(wwn, "5" QUMRANET_OUI "%09" PRIx64, + virRandomBits(36)) < 0) { + virReportOOMError(); + return -1; + } + + return 0; +} diff --git a/src/util/virrandom.h b/src/util/virrandom.h index e180a2f..443451e 100644 --- a/src/util/virrandom.h +++ b/src/util/virrandom.h @@ -27,5 +27,6 @@ int virRandomInitialize(uint32_t seed) ATTRIBUTE_RETURN_CHECK; uint64_t virRandomBits(int nbits); +int virRandomGenerateWWN(char **wwn); #endif /* __VIR_RANDOM_H__ */ -- 1.7.7.3

On 02/06/2012 02:11 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And use Qumranet's OUI (00:1A:4A) as the 3-byte vendor indentifier. The last 36 bits are auto-generated. --- src/conf/node_device_conf.c | 36 +++++++++++++++++++----------------- src/libvirt_private.syms | 1 + src/util/virrandom.c | 18 ++++++++++++++++++ src/util/virrandom.h | 1 + 4 files changed, 39 insertions(+), 17 deletions(-)
+ +#define QUMRANET_OUI "001a4a" + +int +virRandomGenerateWWN(char **wwn) { + if (virAsprintf(wwn, "5" QUMRANET_OUI "%09" PRIx64, + virRandomBits(36)) < 0) { + virReportOOMError(); + return -1; + } + + return 0;
ACK. It looks a lot shorter than v4, which is a good thing :) -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2012/2/6 Eric Blake <eblake@redhat.com>:
On 02/06/2012 02:11 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And use Qumranet's OUI (00:1A:4A) as the 3-byte vendor indentifier. The last 36 bits are auto-generated. --- src/conf/node_device_conf.c | 36 +++++++++++++++++++----------------- src/libvirt_private.syms | 1 + src/util/virrandom.c | 18 ++++++++++++++++++ src/util/virrandom.h | 1 + 4 files changed, 39 insertions(+), 17 deletions(-)
+ +#define QUMRANET_OUI "001a4a" + +int +virRandomGenerateWWN(char **wwn) { + if (virAsprintf(wwn, "5" QUMRANET_OUI "%09" PRIx64, + virRandomBits(36)) < 0) { + virReportOOMError(); + return -1; + } + + return 0;
ACK. It looks a lot shorter than v4, which is a good thing :)
Maybe I miss something here because I didn't follow the whole version history, but doesn't this result in using Qumranet's OUI for all generated WWN's in libvirt, also for ESX and Hyper-V (assume the drivers would support node devices yet) that might use different OUIs? Or do I misunderstand this? -- Matthias Bolte http://photron.blogspot.com

On 2012年02月07日 03:13, Matthias Bolte wrote:
2012/2/6 Eric Blake<eblake@redhat.com>:
On 02/06/2012 02:11 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And use Qumranet's OUI (00:1A:4A) as the 3-byte vendor indentifier. The last 36 bits are auto-generated. --- src/conf/node_device_conf.c | 36 +++++++++++++++++++----------------- src/libvirt_private.syms | 1 + src/util/virrandom.c | 18 ++++++++++++++++++ src/util/virrandom.h | 1 + 4 files changed, 39 insertions(+), 17 deletions(-)
+ +#define QUMRANET_OUI "001a4a" + +int +virRandomGenerateWWN(char **wwn) { + if (virAsprintf(wwn, "5" QUMRANET_OUI "%09" PRIx64, + virRandomBits(36))< 0) { + virReportOOMError(); + return -1; + } + + return 0;
ACK. It looks a lot shorter than v4, which is a good thing :)
Maybe I miss something here because I didn't follow the whole version history, but doesn't this result in using Qumranet's OUI for all generated WWN's in libvirt, also for ESX and Hyper-V (assume the drivers would support node devices yet) that might use different OUIs? Or do I misunderstand this?
Yeah, that's strange to see Qumranet's in ESX or other else environment, I created v6 to choose OUI according to the underlying hypervisor driver. Thanks for pointing it out. Regards, Osier

The auto-generated WWN comply with the new addressing schema of WWN: <quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote> We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated. --- src/conf/node_device_conf.c | 81 +++++++++++++++++++++------------ src/conf/node_device_conf.h | 9 +++- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 4 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 8 ++-- src/util/virrandom.c | 53 ++++++++++++++++++++++ src/util/virrandom.h | 1 + src/xen/xen_driver.c | 2 +- 9 files changed, 121 insertions(+), 40 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index d9dc9ac..d1befe0 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -37,6 +37,8 @@ #include "buf.h" #include "uuid.h" #include "pci.h" +#include "virrandom.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -63,19 +65,12 @@ VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, static int virNodeDevCapsDefParseString(const char *xpath, xmlXPathContextPtr ctxt, - char **string, - virNodeDeviceDefPtr def, - const char *missing_error_fmt) + char **string) { char *s; - s = virXPathString(xpath, ctxt); - if (s == NULL) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - missing_error_fmt, - def->name); + if (!(s = virXPathString(xpath, ctxt))) return -1; - } *string = s; return 0; @@ -717,7 +712,8 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, union _virNodeDevCapData *data, - int create) + int create, + const char *virt_type) { xmlNodePtr orignode, *nodes = NULL; int ret = -1, n = 0, i; @@ -763,20 +759,31 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, if (virNodeDevCapsDefParseString("string(./wwnn[1])", ctxt, - &data->scsi_host.wwnn, - def, - _("no WWNN supplied for '%s'")) < 0) { - goto out; + &data->scsi_host.wwnn) < 0) { + if (virRandomGenerateWWN(&data->scsi_host.wwnn, virt_type) < 0) { + virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, + _("no WWNN supplied for '%s', and " + "auto-generation failed"), + def->name); + goto out; + } } if (virNodeDevCapsDefParseString("string(./wwpn[1])", ctxt, - &data->scsi_host.wwpn, - def, - _("no WWPN supplied for '%s'")) < 0) { - goto out; + &data->scsi_host.wwpn) < 0) { + if (virRandomGenerateWWN(&data->scsi_host.wwpn, virt_type) < 0) { + virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, + _("no WWPN supplied for '%s', and " + "auto-generation failed"), + def->name); + goto out; + } } ctxt->node = orignode2; } else { @@ -1060,7 +1067,8 @@ static virNodeDevCapsDefPtr virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - int create) + int create, + const char *virt_type) { virNodeDevCapsDefPtr caps; char *tmp; @@ -1104,7 +1112,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, ret = virNodeDevCapNetParseXML(ctxt, def, node, &caps->data); break; case VIR_NODE_DEV_CAP_SCSI_HOST: - ret = virNodeDevCapScsiHostParseXML(ctxt, def, node, &caps->data, create); + ret = virNodeDevCapScsiHostParseXML(ctxt, def, node, + &caps->data, + create, + virt_type); break; case VIR_NODE_DEV_CAP_SCSI_TARGET: ret = virNodeDevCapScsiTargetParseXML(ctxt, def, node, &caps->data); @@ -1133,7 +1144,9 @@ error: } static virNodeDeviceDefPtr -virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create) +virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, + int create, + const char *virt_type) { virNodeDeviceDefPtr def; virNodeDevCapsDefPtr *next_cap; @@ -1180,7 +1193,10 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create) next_cap = &def->caps; for (i = 0 ; i < n ; i++) { - *next_cap = virNodeDevCapsDefParseXML(ctxt, def, nodes[i], create); + *next_cap = virNodeDevCapsDefParseXML(ctxt, def, + nodes[i], + create, + virt_type); if (!*next_cap) { VIR_FREE(nodes); goto error; @@ -1200,7 +1216,8 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create) virNodeDeviceDefPtr virNodeDeviceDefParseNode(xmlDocPtr xml, xmlNodePtr root, - int create) + int create, + const char *virt_type) { xmlXPathContextPtr ctxt = NULL; virNodeDeviceDefPtr def = NULL; @@ -1220,7 +1237,7 @@ virNodeDeviceDefParseNode(xmlDocPtr xml, } ctxt->node = root; - def = virNodeDeviceDefParseXML(ctxt, create); + def = virNodeDeviceDefParseXML(ctxt, create, virt_type); cleanup: xmlXPathFreeContext(ctxt); @@ -1230,13 +1247,15 @@ cleanup: static virNodeDeviceDefPtr virNodeDeviceDefParse(const char *str, const char *filename, - int create) + int create, + const char *virt_type) { xmlDocPtr xml; virNodeDeviceDefPtr def = NULL; if ((xml = virXMLParse(filename, str, _("(node_device_definition)")))) { - def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), create); + def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), + create, virt_type); xmlFreeDoc(xml); } @@ -1245,16 +1264,18 @@ virNodeDeviceDefParse(const char *str, virNodeDeviceDefPtr virNodeDeviceDefParseString(const char *str, - int create) + int create, + const char *virt_type) { - return virNodeDeviceDefParse(str, NULL, create); + return virNodeDeviceDefParse(str, NULL, create, virt_type); } virNodeDeviceDefPtr virNodeDeviceDefParseFile(const char *filename, - int create) + int create, + const char *virt_type) { - return virNodeDeviceDefParse(NULL, filename, create); + return virNodeDeviceDefParse(NULL, filename, create, virt_type); } /* diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index a1833a0..4aaf4c8 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -234,12 +234,15 @@ void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs, char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def); virNodeDeviceDefPtr virNodeDeviceDefParseString(const char *str, - int create); + int create, + const char *virt_type); virNodeDeviceDefPtr virNodeDeviceDefParseFile(const char *filename, - int create); + int create, + const char *virt_type); virNodeDeviceDefPtr virNodeDeviceDefParseNode(xmlDocPtr xml, xmlNodePtr root, - int create); + int create, + const char *virt_type); int virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, char **wwnn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6ad36c..421986b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1373,6 +1373,7 @@ virPidFileDeletePath; # virrandom.h virRandomBits; +virRandomGenerateWWN; virRandomInitialize; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 681655e..b0a29cd 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -558,12 +558,14 @@ nodeDeviceCreateXML(virConnectPtr conn, char *wwnn = NULL, *wwpn = NULL; int parent_host = -1; virNodeDevicePtr dev = NULL; + const char *virt_type = NULL; virCheckFlags(0, NULL); + virt_type = virConnectGetType(conn); nodeDeviceLock(driver); - def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE); + def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type); if (def == NULL) { goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3f940e8..464fd7d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9039,7 +9039,7 @@ qemudNodeDeviceGetPciInfo (virNodeDevicePtr dev, if (!xml) goto out; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE); + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); if (!def) goto out; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bf6b148..39fd63d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -608,7 +608,7 @@ static int testOpenDefault(virConnectPtr conn) { virStoragePoolObjUnlock(poolobj); /* Init default node device */ - if (!(nodedef = virNodeDeviceDefParseString(defaultNodeXML, 0))) + if (!(nodedef = virNodeDeviceDefParseString(defaultNodeXML, 0, NULL))) goto error; if (!(nodeobj = virNodeDeviceAssignDef(&privconn->devs, nodedef))) { @@ -1057,12 +1057,12 @@ static int testOpenFromFile(virConnectPtr conn, goto error; } - def = virNodeDeviceDefParseFile(absFile, 0); + def = virNodeDeviceDefParseFile(absFile, 0, NULL); VIR_FREE(absFile); if (!def) goto error; } else { - if ((def = virNodeDeviceDefParseNode(xml, devs[i], 0)) == NULL) + if ((def = virNodeDeviceDefParseNode(xml, devs[i], 0, NULL)) == NULL) goto error; } if (!(dev = virNodeDeviceAssignDef(&privconn->devs, def))) { @@ -5285,7 +5285,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, testDriverLock(driver); - def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE); + def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL); if (def == NULL) { goto cleanup; } diff --git a/src/util/virrandom.c b/src/util/virrandom.c index ec0cf03..760b975 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -22,10 +22,20 @@ #include <config.h> #include <stdlib.h> +#include <inttypes.h> #include "virrandom.h" #include "threads.h" #include "count-one-bits.h" +#include "util.h" +#include "virterror_internal.h" +#include "conf/domain_conf.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +#define virRandomError(code, ...) \ + virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) static char randomState[128]; static struct random_data randomData; @@ -79,3 +89,46 @@ uint64_t virRandomBits(int nbits) virMutexUnlock(&randomLock); return ret; } + +#define QUMRANET_OUI "001a4a" +#define VMWARE_OUI "000569" +#define MICROSOFT_OUI "0050f2" +#define XEN_OUI "00163e" + +int +virRandomGenerateWWN(char **wwn, + const char *virt_type) { + const char *oui = NULL; + + if (!virt_type) { + virRandomError(VIR_ERR_INVALID_ARG, "%s", + _("argument virt_type must not be NULL")); + return -1; + } + + if (STREQ(virt_type, "QEMU")) { + oui = QUMRANET_OUI; + } else if (STREQ(virt_type, "Xen") || + STREQ(virt_type, "xenlight") || + STREQ(virt_type, "XenAPI")) { + oui = XEN_OUI; + } else if (STREQ(virt_type, "ESX") || + STREQ(virt_type, "VMWARE")) { + oui = VMWARE_OUI; + } else if (STREQ(virt_type, "HYPER-V")) { + oui = MICROSOFT_OUI; + } else { + virRandomError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unsupported virt type")); + return -1; + } + + if (virAsprintf(wwn, "5" "%s%09" PRIx64, + oui, virRandomBits(36)) < 0) { + virReportOOMError(); + return -1; + } + + return 0; +} diff --git a/src/util/virrandom.h b/src/util/virrandom.h index e180a2f..24ab0b1 100644 --- a/src/util/virrandom.h +++ b/src/util/virrandom.h @@ -27,5 +27,6 @@ int virRandomInitialize(uint32_t seed) ATTRIBUTE_RETURN_CHECK; uint64_t virRandomBits(int nbits); +int virRandomGenerateWWN(char **wwn, const char *virt_type); #endif /* __VIR_RANDOM_H__ */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 94cafde..635f468 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1950,7 +1950,7 @@ xenUnifiedNodeDeviceGetPciInfo (virNodeDevicePtr dev, if (!xml) goto out; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE); + def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL); if (!def) goto out; -- 1.7.7.3

2012/2/7 Osier Yang <jyang@redhat.com>:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
A step in the right direction :) But why should we use a string here and call virConnectGetType when we already have a virDomainVirtType enum and the virDomainDef struct has a virtType member that holds the required information? -- Matthias Bolte http://photron.blogspot.com

On 2012年02月07日 21:15, Matthias Bolte wrote:
2012/2/7 Osier Yang<jyang@redhat.com>:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
A step in the right direction :)
But why should we use a string here and call virConnectGetType when we already have a virDomainVirtType enum and the virDomainDef struct has a virtType member that holds the required information?
node device driver is independant with hypervisor driver, and it doesn't relates with a domain, so the only way for us to determine which OUI should be used is to query the connection to get the virt type. The only scenario of using virDomainVirtType in my mind is to extend virNodeDeviceCreateXML to accept a virt type flag, (or new API). and in that case we need to add a new enum like virDomainVirtType in libvirt.h too. Not sure If I understand you correctly though. :) Regards, Osier

On 02/07/2012 06:38 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
+ +#define QUMRANET_OUI "001a4a" +#define VMWARE_OUI "000569" +#define MICROSOFT_OUI "0050f2" +#define XEN_OUI "00163e" + +int +virRandomGenerateWWN(char **wwn, + const char *virt_type) {
I don't like this interface - it makes virrandom.c know too much. A better interface would be: virRandomGenerateWWN(char **wwn, const char *oui) where the caller is responsible for determining the appropriate OUI for the hypervisor to be passed in. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 2012年02月07日 21:29, Eric Blake wrote:
On 02/07/2012 06:38 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
+ +#define QUMRANET_OUI "001a4a" +#define VMWARE_OUI "000569" +#define MICROSOFT_OUI "0050f2" +#define XEN_OUI "00163e" + +int +virRandomGenerateWWN(char **wwn, + const char *virt_type) {
I don't like this interface - it makes virrandom.c know too much. A better interface would be:
virRandomGenerateWWN(char **wwn, const char *oui)
where the caller is responsible for determining the appropriate OUI for the hypervisor to be passed in.
I have thought this, but the problem is we want auto-generate the WWN, the nodedevice driver doesn't known which OUI should be passed to virRandomGenerateWWN in this case, unless we extend the API virNodeDeviceCreateXML to accept a flag, or introduce a new API. I guess we won't want to see this in this period. :-) Regards, Osier

On 2012年02月07日 22:03, Osier Yang wrote:
On 2012年02月07日 21:29, Eric Blake wrote:
On 02/07/2012 06:38 AM, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
+ +#define QUMRANET_OUI "001a4a" +#define VMWARE_OUI "000569" +#define MICROSOFT_OUI "0050f2" +#define XEN_OUI "00163e" + +int +virRandomGenerateWWN(char **wwn, + const char *virt_type) {
I don't like this interface - it makes virrandom.c know too much. A better interface would be:
virRandomGenerateWWN(char **wwn, const char *oui)
where the caller is responsible for determining the appropriate OUI for the hypervisor to be passed in.
I have thought this, but the problem is we want auto-generate the WWN, the nodedevice driver doesn't known which OUI should be passed to virRandomGenerateWWN in this case, unless we extend the API virNodeDeviceCreateXML to accept a flag, or introduce a new API. I guess we won't want to see this in this period. :-)
Regards, Osier
Hi, Eric, How about my excuse, ;-) Osier

On 02/07/2012 07:03 AM, Osier Yang wrote:
I don't like this interface - it makes virrandom.c know too much. A better interface would be:
virRandomGenerateWWN(char **wwn, const char *oui)
where the caller is responsible for determining the appropriate OUI for the hypervisor to be passed in.
I have thought this, but the problem is we want auto-generate the WWN, the nodedevice driver doesn't known which OUI should be passed to virRandomGenerateWWN in this case, unless we extend the API virNodeDeviceCreateXML to accept a flag, or introduce a new API. I guess we won't want to see this in this period. :-)
I'm okay if we ditch this for now. But I guess I still don't understand the motivation behind this patch - what's the original scenario where we are parsing XML where auto-generating a WWN would prove useful? If it is done through a connection, then we know what hypervisor we are targetting, and that hypervisor should have the information on what OUI to prefer. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 2012年02月10日 07:05, Eric Blake wrote:
On 02/07/2012 07:03 AM, Osier Yang wrote:
I don't like this interface - it makes virrandom.c know too much. A better interface would be:
virRandomGenerateWWN(char **wwn, const char *oui)
where the caller is responsible for determining the appropriate OUI for the hypervisor to be passed in.
I have thought this, but the problem is we want auto-generate the WWN, the nodedevice driver doesn't known which OUI should be passed to virRandomGenerateWWN in this case, unless we extend the API virNodeDeviceCreateXML to accept a flag, or introduce a new API. I guess we won't want to see this in this period. :-)
I'm okay if we ditch this for now. But I guess I still don't understand the motivation behind this patch - what's the original scenario where we are parsing XML where auto-generating a WWN would prove useful?
Basically it's the request from customer so that the admins don't have to specify the WWNs manually when using their management apps, If it
is done through a connection, then we know what hypervisor we are targetting, and that hypervisor should have the information on what OUI to prefer.
That's the way this patch goes. :) Thanks, now I pushed the patch. Osier

On Tue, Feb 07, 2012 at 09:38:50PM +0800, Osier Yang wrote:
The auto-generated WWN comply with the new addressing schema of WWN:
<quote> the first nibble is either hex 5 or 6 followed by a 3-byte vendor identifier and 36 bits for a vendor-specified serial number. </quote>
We choose hex 5 for the first nibble. And for the 3-bytes vendor ID, we uses the OUI according to underlying hypervisor type, (invoking virConnectGetType to get the virt type). e.g. If virConnectGetType returns "QEMU", we use Qumranet's OUI (00:1A:4A), if returns ESX|VMWARE, we use VMWARE's OUI (00:05:69). Currently it only supports qemu|xen|libxl|xenapi|hyperv|esx|vmware drivers. The last 36 bits are auto-generated.
diff --git a/src/util/virrandom.c b/src/util/virrandom.c index ec0cf03..760b975 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -22,10 +22,20 @@ #include <config.h>
#include <stdlib.h> +#include <inttypes.h>
#include "virrandom.h" #include "threads.h" #include "count-one-bits.h" +#include "util.h" +#include "virterror_internal.h" +#include "conf/domain_conf.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +#define virRandomError(code, ...) \ + virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__)
static char randomState[128]; static struct random_data randomData; @@ -79,3 +89,46 @@ uint64_t virRandomBits(int nbits) virMutexUnlock(&randomLock); return ret; } + +#define QUMRANET_OUI "001a4a" +#define VMWARE_OUI "000569" +#define MICROSOFT_OUI "0050f2" +#define XEN_OUI "00163e" + +int +virRandomGenerateWWN(char **wwn, + const char *virt_type) { + const char *oui = NULL; + + if (!virt_type) { + virRandomError(VIR_ERR_INVALID_ARG, "%s", + _("argument virt_type must not be NULL")); + return -1; + } + + if (STREQ(virt_type, "QEMU")) { + oui = QUMRANET_OUI; + } else if (STREQ(virt_type, "Xen") || + STREQ(virt_type, "xenlight") || + STREQ(virt_type, "XenAPI")) { + oui = XEN_OUI; + } else if (STREQ(virt_type, "ESX") || + STREQ(virt_type, "VMWARE")) { + oui = VMWARE_OUI; + } else if (STREQ(virt_type, "HYPER-V")) { + oui = MICROSOFT_OUI; + } else { + virRandomError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unsupported virt type")); + return -1; + } + + if (virAsprintf(wwn, "5" "%s%09" PRIx64, + oui, virRandomBits(36)) < 0) { + virReportOOMError(); + return -1; + } + + return 0; +}
This has broken the build on Mingw32, because PRIx64 is coming from the Win32 headers, but virAsprintf uses the gnulib printf. You need to use the normal "%09llu" format specific here, not the PRIx64 macro. We ought to have a syntax check test to forbid use of PRIx* stuff really. 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 :|

It breaks the build on Mingw32, because PRIx64 is coming from the Win32 headers, but virAsprintf uses the gnulib printf. --- src/util/virrandom.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 630bc00..151cf4b 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -123,8 +123,8 @@ virRandomGenerateWWN(char **wwn, return -1; } - if (virAsprintf(wwn, "5" "%s%09" PRIx64, - oui, virRandomBits(36)) < 0) { + if (virAsprintf(wwn, "5" "%s%09llx", oui, + (unsigned long long)virRandomBits(36)) < 0) { virReportOOMError(); return -1; } -- 1.7.7.3

On 2012年02月10日 20:05, Osier Yang wrote:
It breaks the build on Mingw32, because PRIx64 is coming from the Win32 headers, but virAsprintf uses the gnulib printf.
It's pushed under build breaking rule.
--- src/util/virrandom.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 630bc00..151cf4b 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -123,8 +123,8 @@ virRandomGenerateWWN(char **wwn, return -1; }
- if (virAsprintf(wwn, "5" "%s%09" PRIx64, - oui, virRandomBits(36))< 0) { + if (virAsprintf(wwn, "5" "%s%09llx", oui, + (unsigned long long)virRandomBits(36))< 0) { virReportOOMError(); return -1; }
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Matthias Bolte
-
Osier Yang