[libvirt] [PATCH 0/2] Implement nodedev destroy API
Here is a patch that implements the node device destroy API call, as well as the suggestions that Dan made in his feedback on the patch implementing create. The patch also makes it not necessary for the caller of the node device create function to specify a name for the device and a host number for the adapter in the XML. The change to do so is kind of kludgy, though, IMO, as it involved changing the function signature of chain of internal function calls to differentiate a device that is being created from an existing device. I'm torn as to whether I think that's a better solution than requiring the caller of the nodedev create API to specify a name that will be ignored. One benefit of requiring the user to specify the name is that it is used in some error messages, so I can argue it both ways. Opinions very welcome. I'd particularly like someone in the Solaris world to make sure that the code builds as there is some #ifdef __linux__ in the patch. I tried to spin up Opensolaris to try it out but ran up against bug 6784591 and a lack of time. I've also tried to make it relatively easy for someone to add support for non-Linux OSes, so a patch there would be appreciated. Dave
--- src/node_device.c | 355 ++++++++++++++++++++++++++++++++++++++++++++++++ src/node_device.h | 14 ++ src/node_device_conf.c | 106 ++++++++++++++- src/node_device_conf.h | 16 ++ src/node_device_hal.c | 134 ++++++++++++++++++ src/storage_backend.c | 24 +--- src/virsh.c | 53 +++++++ 7 files changed, 678 insertions(+), 24 deletions(-) diff --git a/src/node_device.c b/src/node_device.c index b84729f..25d3251 100644 --- a/src/node_device.c +++ b/src/node_device.c @@ -25,6 +25,8 @@ #include <unistd.h> #include <errno.h> +#include <fcntl.h> +#include <time.h> #include "virterror_internal.h" #include "datatypes.h" @@ -133,6 +135,53 @@ cleanup: return ret; } + +/* Caller must hold the driver lock. */ +static virNodeDevicePtr +nodeDeviceLookupByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn) +{ + unsigned int i, found = 0; + virDeviceMonitorStatePtr driver = conn->devMonPrivateData; + virNodeDeviceObjListPtr devs = &driver->devs; + virNodeDevCapsDefPtr cap = NULL; + virNodeDeviceObjPtr obj = NULL; + virNodeDevicePtr dev = NULL; + + for (i = 0; i < devs->count; i++) { + + obj = devs->objs[i]; + virNodeDeviceObjLock(obj); + cap = obj->def->caps; + + while (cap) { + + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + if (STREQ(cap->data.scsi_host.wwnn, wwnn) && + STREQ(cap->data.scsi_host.wwpn, wwpn)) { + found = 1; + goto out; + } + } + } + cap = cap->next; + } + + virNodeDeviceObjUnlock(obj); + } + +out: + if (found) { + dev = virGetNodeDevice(conn, obj->def->name); + virNodeDeviceObjUnlock(obj); + } + + return dev; +} + + static char *nodeDeviceDumpXML(virNodeDevicePtr dev, unsigned int flags ATTRIBUTE_UNUSED) { @@ -258,6 +307,310 @@ cleanup: } +static int +nodeDeviceVportCreateDelete(virConnectPtr conn, + const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int fd = -1; + int retval = 0; + char *operation_path; + const char *operation_file; + char *vport_name; + size_t towrite = 0; + unsigned int written = 0; + + switch (operation) { + case VPORT_CREATE: + operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX; + break; + case VPORT_DELETE: + operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX; + break; + default: + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid vport operation (%d)"), operation); + retval = -1; + goto no_unwind; + break; + } + + if (virAsprintf(&operation_path, + "%shost%d%s", + LINUX_SYSFS_FC_HOST_PREFIX, + parent_host, + operation_file) < 0) { + + virReportOOMError(conn); + retval = -1; + goto no_unwind; + } + + VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); + + fd = open(operation_path, O_WRONLY); + + if (fd < 0) { + virReportSystemError(conn, errno, + _("Could not open '%s' for vport operation"), + operation_path); + retval = -1; + goto free_path; + } + + if (virAsprintf(&vport_name, + "%s:%s", + wwpn, + wwnn) < 0) { + + virReportOOMError(conn); + retval = -1; + goto close_fd; + } + + towrite = strlen(vport_name); + written = safewrite(fd, vport_name, towrite); + if (written != towrite) { + virReportSystemError(conn, errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed " + "(towrite: %lu written: %d)"), + vport_name, operation_path, + towrite, written); + retval = -1; + } + + VIR_FREE(vport_name); +close_fd: + close(fd); +free_path: + VIR_FREE(operation_path); +no_unwind: + VIR_DEBUG("%s", _("Vport operation complete")); + return retval; +} + + +static int +get_wwns(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn) +{ + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + cap = def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + *wwnn = cap->data.scsi_host.wwnn; + *wwpn = cap->data.scsi_host.wwnn; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + /* XXX This error code is wrong--it results in errors of the form: + "error: invalid node device pointer in Device foo is not a fibre channel HBA" + */ + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Device %s is not a fibre channel HBA"), + def->name); + ret = -1; + } + + return ret; +} + + +static int +get_parent_host(virConnectPtr conn, + virDeviceMonitorStatePtr driver, + virNodeDeviceDefPtr def, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(&driver->devs, def->parent); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Could not find parent device for '%s'"), + def->name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Device %s is not a SCSI host"), + parent->def->name); + ret = -1; + } + +out: + if (parent != NULL) { + virNodeDeviceObjUnlock(parent); + } + + return ret; +} + + +static virNodeDevicePtr +find_new_device(virConnectPtr conn, const char *wwnn, const char *wwpn) +{ + virNodeDevicePtr dev = NULL; + virDeviceMonitorStatePtr driver = conn->devMonPrivateData; + time_t start = 0, now = 0; + + /* When large numbers of devices are present on the host, it's + * possible for udev not to realize that it has work to do before + * we get here. We thus keep trying to find the new device we + * just created for up to LINUX_NEW_DEVICE_WAIT_TIME. Note that + * udev's default settle time is 180 seconds, so once udev + * realizes that it has work to do, it might take that long for + * the udev wait to return. Thus the total maximum time for this + * function to return is the udev settle time plus + * LINUX_NEW_DEVICE_WAIT_TIME. + * + * This whole area is generally racy, but if we retry the udev + * wait for LINUX_NEW_DEVICE_WAIT_TIME seconds and there's still + * no device, it's probably safe to assume it's not going to + * appear. */ + start = time(NULL); + if (start == (time_t)-1) { + virNodeDeviceReportError(dev->conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not get current time")); + /* Set start time to the epoch so we try find the device + * once. */ + start = 0; + } + + while (dev == NULL && now - start < LINUX_NEW_DEVICE_WAIT_TIME) { + /* We can't hold the driver lock while we wait because the + wait for devices call takes it. It's safe to drop the lock + because we're done with the driver structure at this point + anyway. We take it again when we look to see what, if + anything, was created. */ + nodeDeviceUnlock(driver); + virNodeDeviceWaitForDevices(conn); + nodeDeviceLock(driver); + + dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn); + + if (dev == NULL) { + sleep(5); + now = time(NULL); + if (now == (time_t)-1) { + virNodeDeviceReportError(dev->conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not get current time")); + break; + } + } + } + + return dev; +} + +static virNodeDevicePtr +nodeDeviceCreateXML(virConnectPtr conn, + const char *xmlDesc, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virDeviceMonitorStatePtr driver = conn->devMonPrivateData; + virNodeDeviceDefPtr def = NULL; + char *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + virNodeDevicePtr dev = NULL; + + nodeDeviceLock(driver); + + def = virNodeDeviceDefParseString(conn, xmlDesc); + if (def == NULL) { + goto cleanup; + } + + if (get_wwns(conn, def, &wwnn, &wwpn) == -1) { + goto cleanup; + } + + if (get_parent_host(conn, driver, def, &parent_host) == -1) { + goto cleanup; + } + + if (nodeDeviceVportCreateDelete(conn, + parent_host, + wwpn, + wwnn, + VPORT_CREATE) == -1) { + goto cleanup; + } + + dev = find_new_device(conn, wwnn, wwpn); + /* We don't check the return value, because one way or another, + * we're returning what we get... */ + + if (dev == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_NO_NODE_DEVICE, NULL); + } + +cleanup: + nodeDeviceUnlock(driver); + virNodeDeviceDefFree(def); + return dev; +} + + +static int +nodeDeviceDestroy(virNodeDevicePtr dev ATTRIBUTE_UNUSED) +{ + return 0; +} + + +#if defined(UDEVADM) || defined(UDEVSETTLE) +void virNodeDeviceWaitForDevices(virConnectPtr conn) +{ +#ifdef UDEVADM + const char *const settleprog[] = { UDEVADM, "settle", NULL }; +#else + const char *const settleprog[] = { UDEVSETTLE, NULL }; +#endif + int exitstatus; + + if (access(settleprog[0], X_OK) != 0) + return; + + /* + * NOTE: we ignore errors here; this is just to make sure that any device + * nodes that are being created finish before we try to scan them. + * If this fails for any reason, we still have the backup of polling for + * 5 seconds for device nodes. + */ + virRun(conn, settleprog, &exitstatus); +} +#else +void virNodeDeviceWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {} +#endif + + void registerCommonNodeFuncs(virDeviceMonitorPtr driver) { driver->numOfDevices = nodeNumOfDevices; @@ -267,6 +620,8 @@ void registerCommonNodeFuncs(virDeviceMonitorPtr driver) driver->deviceGetParent = nodeDeviceGetParent; driver->deviceNumOfCaps = nodeDeviceNumOfCaps; driver->deviceListCaps = nodeDeviceListCaps; + driver->deviceCreateXML = nodeDeviceCreateXML; + driver->deviceDestroy = nodeDeviceDestroy; } diff --git a/src/node_device.h b/src/node_device.h index 9496120..59cd5f5 100644 --- a/src/node_device.h +++ b/src/node_device.h @@ -28,6 +28,18 @@ #include "driver.h" #include "node_device_conf.h" + +#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host" +#define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" +#define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/" + +#define VPORT_CREATE 0 +#define VPORT_DELETE 1 +#define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create" +#define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete" + +#define LINUX_NEW_DEVICE_WAIT_TIME 60 + #ifdef HAVE_HAL int halNodeRegister(void); #endif @@ -42,4 +54,6 @@ void registerCommonNodeFuncs(virDeviceMonitorPtr mon); int nodedevRegister(void); +void virNodeDeviceWaitForDevices(virConnectPtr conn); + #endif /* __VIR_NODE_DEVICE_H__ */ diff --git a/src/node_device_conf.c b/src/node_device_conf.c index 6e04112..6c74551 100644 --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -53,9 +53,34 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", "80211") +VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, + "fc_host", + "vport_ops") #define virNodeDeviceLog(msg...) fprintf(stderr, msg) +static int +virNodeDevCapsDefParseString(virConnectPtr conn, + const char *xpath, + xmlXPathContextPtr ctxt, + char **string, + virNodeDeviceDefPtr def, + const char *missing_error_fmt) +{ + char *s; + + s = virXPathString(conn, xpath, ctxt); + if (s == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + missing_error_fmt, + def->name); + return -1; + } + + *string = s; + return 0; +} + virNodeDeviceObjPtr virNodeDeviceFindByName(const virNodeDeviceObjListPtr devs, const char *name) { @@ -302,6 +327,18 @@ char *virNodeDeviceDefFormat(virConnectPtr conn, case VIR_NODE_DEV_CAP_SCSI_HOST: virBufferVSprintf(&buf, " <host>%d</host>\n", data->scsi_host.host); + if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + virBufferAddLit(&buf, " <capability type='fc_host'>\n"); + virBufferVSprintf(&buf, + " <wwnn>%s</wwnn>\n", data->scsi_host.wwnn); + virBufferVSprintf(&buf, + " <wwpn>%s</wwpn>\n", data->scsi_host.wwpn); + virBufferAddLit(&buf, " </capability>\n"); + } + if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { + virBufferAddLit(&buf, " <capability type='vport_ops' />\n"); + } + break; case VIR_NODE_DEV_CAP_SCSI: virBufferVSprintf(&buf, " <host>%d</host>\n", data->scsi.host); @@ -563,8 +600,9 @@ virNodeDevCapScsiHostParseXML(virConnectPtr conn, xmlNodePtr node, union _virNodeDevCapData *data) { - xmlNodePtr orignode; - int ret = -1; + xmlNodePtr orignode, *nodes = NULL; + int ret = -1, n = 0, i; + char *type = NULL; orignode = ctxt->node; ctxt->node = node; @@ -572,15 +610,77 @@ virNodeDevCapScsiHostParseXML(virConnectPtr conn, if (virNodeDevCapsDefParseULong(conn, "number(./host[1])", ctxt, &data->scsi_host.host, def, _("no SCSI host ID supplied for '%s'"), - _("invalid SCSI host ID supplied for '%s'")) < 0) + _("invalid SCSI host ID supplied for '%s'")) < 0) { + goto out; + } + + if ((n = virXPathNodeSet(conn, "./capability", ctxt, &nodes)) < 0) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("error parsing SCSI host capabilities for '%s'"), + def->name); goto out; + } + + for (i = 0 ; i < n ; i++) { + type = virXMLPropString(nodes[i], "type"); + + if (!type) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("missing SCSI host capability type for '%s'"), + def->name); + goto out; + } + + if (STREQ(type, "vport_ops")) { + + data->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + + } else if (STREQ(type, "fc_host")) { + + xmlNodePtr orignode2; + + data->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + orignode2 = ctxt->node; + ctxt->node = nodes[i]; + + if (virNodeDevCapsDefParseString(conn, "string(./wwnn[1])", + ctxt, + &data->scsi_host.wwnn, + def, + _("no WWNN supplied for '%s'")) < 0) { + goto out; + } + + if (virNodeDevCapsDefParseString(conn, "string(./wwpn[1])", + ctxt, + &data->scsi_host.wwpn, + def, + _("no WWPN supplied for '%s'")) < 0) { + goto out; + } + + ctxt->node = orignode2; + + } else { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown SCSI host capability type '%s' for '%s'"), + type, def->name); + goto out; + } + + VIR_FREE(type); + } ret = 0; + out: + VIR_FREE(type); ctxt->node = orignode; return ret; } + static int virNodeDevCapNetParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, diff --git a/src/node_device_conf.h b/src/node_device_conf.h index 26e5558..8233a91 100644 --- a/src/node_device_conf.h +++ b/src/node_device_conf.h @@ -48,8 +48,16 @@ enum virNodeDevNetCapType { VIR_NODE_DEV_CAP_NET_LAST }; +enum virNodeDevHBACapType { + /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ + VIR_NODE_DEV_CAP_HBA_FC_HOST, /* fibre channel HBA */ + VIR_NODE_DEV_CAP_HBA_VPORT_OPS, /* capable of vport operations */ + VIR_NODE_DEV_CAP_HBA_LAST +}; + VIR_ENUM_DECL(virNodeDevCap) VIR_ENUM_DECL(virNodeDevNetCap) +VIR_ENUM_DECL(virNodeDevHBACap) enum virNodeDevStorageCapFlags { VIR_NODE_DEV_CAP_STORAGE_REMOVABLE = (1 << 0), @@ -57,6 +65,11 @@ enum virNodeDevStorageCapFlags { VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE = (1 << 2), }; +enum virNodeDevScsiHostCapFlags { + VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST = (1 << 0), + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS = (1 << 1), +}; + typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { @@ -108,6 +121,9 @@ struct _virNodeDevCapsDef { } net; struct { unsigned host; + char *wwnn; + char *wwpn; + unsigned flags; } scsi_host; struct { unsigned host; diff --git a/src/node_device_hal.c b/src/node_device_hal.c index b214f60..5e54044 100644 --- a/src/node_device_hal.c +++ b/src/node_device_hal.c @@ -26,6 +26,7 @@ #include <stdio.h> #include <stdlib.h> #include <libhal.h> +#include <fcntl.h> #include "node_device_conf.h" #include "virterror_internal.h" @@ -37,6 +38,8 @@ #include "logging.h" #include "node_device.h" +#define VIR_FROM_THIS VIR_FROM_NODEDEV + /* * Host device enumeration (HAL implementation) */ @@ -211,10 +214,141 @@ static int gather_net_cap(LibHalContext *ctx, const char *udi, } +static int check_fc_host(union _virNodeDevCapData *d) +{ + char *sysfs_path = NULL; + char *wwnn_path = NULL; + char *wwpn_path = NULL; + char *p = NULL; + int fd = -1; + char buf[64]; + struct stat st; + + VIR_DEBUG(_("Checking if host%d is an FC HBA"), d->scsi_host.host); + + if (virAsprintf(&sysfs_path, "%s/host%d", + LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host) < 0) { + virReportOOMError(NULL); + goto out; + } + + if (stat(sysfs_path, &st) != 0) { + /* Not an FC HBA */ + goto out; + } + + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + if (virAsprintf(&wwnn_path, "%s/node_name", + sysfs_path) < 0) { + virReportOOMError(NULL); + goto out; + } + + if ((fd = open(wwnn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out; + } + + close(fd); + + p = strstr(buf, "0x"); + if (p != NULL) { + p += strlen("0x"); + } else { + p = buf; + } + + d->scsi_host.wwnn = strndup(p, sizeof(buf)); + if (d->scsi_host.wwnn == NULL) { + virReportOOMError(NULL); + goto out; + } + + p = strchr(d->scsi_host.wwnn, '\n'); + if (p != NULL) { + *p = '\0'; + } + + if (virAsprintf(&wwpn_path, "%s/port_name", + sysfs_path) < 0) { + virReportOOMError(NULL); + goto out; + } + + if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out; + } + + close(fd); + + p = strstr(buf, "0x"); + if (p != NULL) { + p += strlen("0x"); + } else { + p = buf; + } + + d->scsi_host.wwpn = strndup(p, sizeof(buf)); + if (d->scsi_host.wwpn == NULL) { + virReportOOMError(NULL); + goto out; + } + + p = strchr(d->scsi_host.wwpn, '\n'); + if (p != NULL) { + *p = '\0'; + } + +out: + VIR_FREE(sysfs_path); + VIR_FREE(wwnn_path); + VIR_FREE(wwpn_path); + return 0; +} + + +static int check_vport_capable(union _virNodeDevCapData *d) +{ + char *sysfs_path = NULL; + struct stat st; + + if (virAsprintf(&sysfs_path, "%s/host%d/vport_create", + LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host) < 0) { + virReportOOMError(NULL); + goto out; + } + + if (stat(sysfs_path, &st) != 0) { + /* Not a vport capable HBA */ + goto out; + } + + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + +out: + VIR_FREE(sysfs_path); + return 0; +} + + static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, union _virNodeDevCapData *d) { (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); + (void)check_fc_host(d); + (void)check_vport_capable(d); return 0; } diff --git a/src/storage_backend.c b/src/storage_backend.c index b154140..74759cf 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -46,6 +46,7 @@ #include "virterror_internal.h" #include "util.h" #include "memory.h" +#include "node_device.h" #include "storage_backend.h" @@ -245,30 +246,11 @@ virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn, return 0; } -#if defined(UDEVADM) || defined(UDEVSETTLE) void virStorageBackendWaitForDevices(virConnectPtr conn) { -#ifdef UDEVADM - const char *const settleprog[] = { UDEVADM, "settle", NULL }; -#else - const char *const settleprog[] = { UDEVSETTLE, NULL }; -#endif - int exitstatus; - - if (access(settleprog[0], X_OK) != 0) - return; - - /* - * NOTE: we ignore errors here; this is just to make sure that any device - * nodes that are being created finish before we try to scan them. - * If this fails for any reason, we still have the backup of polling for - * 5 seconds for device nodes. - */ - virRun(conn, settleprog, &exitstatus); + virNodeDeviceWaitForDevices(conn); + return; } -#else -void virStorageBackendWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {} -#endif /* * Given a volume path directly in /dev/XXX, iterate over the diff --git a/src/virsh.c b/src/virsh.c index 2e41c02..1a094eb 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -2962,6 +2962,58 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd) /* + * "nodedev-create" command + */ +static const vshCmdInfo info_node_device_create[] = { + {"help", gettext_noop("create a device defined " + "by an XML file on the node")}, + {"desc", gettext_noop("Create a device on the node. Note that this " + "command creates devices on the physical host " + "that can then be assigned to a virtual machine.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_node_device_create[] = { + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, + gettext_noop("file containing an XML description of the device")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr dev = NULL; + char *from; + int found; + int ret = TRUE; + char *buffer; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + from = vshCommandOptString(cmd, "file", &found); + if (!found) + return FALSE; + + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + return FALSE; + + dev = virNodeDeviceCreateXML(ctl->conn, buffer, 0); + free (buffer); + + if (dev != NULL) { + vshPrint(ctl, _("Node device %s created from %s\n"), + virNodeDeviceGetName(dev), from); + } else { + vshError(ctl, FALSE, _("Failed to create node device from %s"), from); + ret = FALSE; + } + + return ret; +} + + +/* * XML Building helper for pool-define-as and pool-create-as */ static const vshCmdOptDef opts_pool_X_as[] = { @@ -5895,6 +5947,7 @@ static const vshCmdDef commands[] = { {"nodedev-dettach", cmdNodeDeviceDettach, opts_node_device_dettach, info_node_device_dettach}, {"nodedev-reattach", cmdNodeDeviceReAttach, opts_node_device_reattach, info_node_device_reattach}, {"nodedev-reset", cmdNodeDeviceReset, opts_node_device_reset, info_node_device_reset}, + {"nodedev-create", cmdNodeDeviceCreate, opts_node_device_create, info_node_device_create}, {"pool-autostart", cmdPoolAutostart, opts_pool_autostart, info_pool_autostart}, {"pool-build", cmdPoolBuild, opts_pool_build, info_pool_build}, -- 1.6.0.6
On Fri, May 08, 2009 at 05:41:08PM -0400, David Allan wrote:
+/* Caller must hold the driver lock. */ +static virNodeDevicePtr +nodeDeviceLookupByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn) +{ + unsigned int i, found = 0; + virDeviceMonitorStatePtr driver = conn->devMonPrivateData; + virNodeDeviceObjListPtr devs = &driver->devs; + virNodeDevCapsDefPtr cap = NULL; + virNodeDeviceObjPtr obj = NULL; + virNodeDevicePtr dev = NULL; + + for (i = 0; i < devs->count; i++) { + + obj = devs->objs[i]; + virNodeDeviceObjLock(obj); + cap = obj->def->caps; + + while (cap) { + + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + if (STREQ(cap->data.scsi_host.wwnn, wwnn) && + STREQ(cap->data.scsi_host.wwpn, wwpn)) {
I would fetch dev and call virNodeDeviceObjUnlock(obj) here before getting to out: i.e. keep the crucial actions closer to the spot and stay generic in the exit section.
+ found = 1; + goto out; + } + } + } + cap = cap->next; + } + + virNodeDeviceObjUnlock(obj); + } + +out: + if (found) { + dev = virGetNodeDevice(conn, obj->def->name); + virNodeDeviceObjUnlock(obj); + } + + return dev; +} + + static char *nodeDeviceDumpXML(virNodeDevicePtr dev, unsigned int flags ATTRIBUTE_UNUSED) { @@ -258,6 +307,310 @@ cleanup: }
+static int +nodeDeviceVportCreateDelete(virConnectPtr conn, + const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int fd = -1; + int retval = 0; + char *operation_path;
I would initilize it to NULL
+ const char *operation_file; + char *vport_name; + size_t towrite = 0; + unsigned int written = 0; + + switch (operation) { + case VPORT_CREATE: + operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX; + break; + case VPORT_DELETE: + operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX; + break; + default: + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid vport operation (%d)"), operation); + retval = -1; + goto no_unwind; + break; + } + + if (virAsprintf(&operation_path, + "%shost%d%s", + LINUX_SYSFS_FC_HOST_PREFIX, + parent_host, + operation_file) < 0) { + + virReportOOMError(conn); + retval = -1; + goto no_unwind; + } + + VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); + + fd = open(operation_path, O_WRONLY); + + if (fd < 0) { + virReportSystemError(conn, errno, + _("Could not open '%s' for vport operation"), + operation_path); + retval = -1; + goto free_path; + } + + if (virAsprintf(&vport_name, + "%s:%s", + wwpn, + wwnn) < 0) { + + virReportOOMError(conn); + retval = -1; + goto close_fd; + } + + towrite = strlen(vport_name); + written = safewrite(fd, vport_name, towrite); + if (written != towrite) { + virReportSystemError(conn, errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed " + "(towrite: %lu written: %d)"), + vport_name, operation_path, + towrite, written); + retval = -1; + } + + VIR_FREE(vport_name); +close_fd: + close(fd); +free_path: + VIR_FREE(operation_path); +no_unwind: + VIR_DEBUG("%s", _("Vport operation complete")); + return retval; +}
and unify the 3 exit labels with if (fd >= 0) close(fd) VIR_FREE(operation_path); VIR_DEBUG(...) if you initilize the variable on enter like fd you can have a single exit point, simpler code, less prone to breakage if the code is modified later [...]
+static int +get_parent_host(virConnectPtr conn, + virDeviceMonitorStatePtr driver, + virNodeDeviceDefPtr def, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(&driver->devs, def->parent); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Could not find parent device for '%s'"), + def->name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Device %s is not a SCSI host"), + parent->def->name); + ret = -1; + } + +out: + if (parent != NULL) { + virNodeDeviceObjUnlock(parent); + }
again I would move it before the break and replace the break to a goto out: I find a bit weird to have only unlocking in this function, but I assume it's just because we get a locked parent as a result of FindByName
+ + return ret; +} + [...] + while (dev == NULL && now - start < LINUX_NEW_DEVICE_WAIT_TIME) {
better to fully parenthesize
+ /* We can't hold the driver lock while we wait because the + wait for devices call takes it. It's safe to drop the lock + because we're done with the driver structure at this point + anyway. We take it again when we look to see what, if + anything, was created. */ + nodeDeviceUnlock(driver); + virNodeDeviceWaitForDevices(conn); + nodeDeviceLock(driver); + + dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn); + + if (dev == NULL) { + sleep(5);
Hum, we do sleep with the lock here, and for a long time !
+ now = time(NULL); + if (now == (time_t)-1) { + virNodeDeviceReportError(dev->conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not get current time")); + break; + } + } + } + + return dev; +} [...] --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -53,9 +53,34 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", "80211")
+VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, + "fc_host", + "vport_ops")
#define virNodeDeviceLog(msg...) fprintf(stderr, msg)
errr, can we fix that while modifying this module, we really ought to use the normal logging internal APIs Well there are other places: network_driver.c:#define networkLog(level, msg...) fprintf(stderr, msg) node_device_conf.c:#define virNodeDeviceLog(msg...) fprintf(stderr, msg) storage_conf.c:#define virStorageLog(msg...) fprintf(stderr, msg) storage_driver.c:#define storageLog(msg...) fprintf(stderr, msg) I will rather do a separate PATCH to plug those...
+ if ((fd = open(wwnn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out;
here fd is open but out: doesn't close it.
+ } + + close(fd);
just set back fd to -1 here and in other places where you close it and in out: add if (fd >= 0) close(fd);
+ if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out;
same here. Alternatively just close(fd) before goto out; in those 2 cases.
static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, union _virNodeDevCapData *d) { (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); + (void)check_fc_host(d); + (void)check_vport_capable(d); return 0; }
Hum, I find hard to justify those void casts One thing I find missing is that we extend the capabilities but I don't see any associated documentation, or is that already covered ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
On Fri, May 08, 2009 at 05:41:08PM -0400, David Allan wrote:
+/* Caller must hold the driver lock. */ +static virNodeDevicePtr +nodeDeviceLookupByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn) +{ + unsigned int i, found = 0; + virDeviceMonitorStatePtr driver = conn->devMonPrivateData; + virNodeDeviceObjListPtr devs = &driver->devs; + virNodeDevCapsDefPtr cap = NULL; + virNodeDeviceObjPtr obj = NULL; + virNodeDevicePtr dev = NULL; + + for (i = 0; i < devs->count; i++) { + + obj = devs->objs[i]; + virNodeDeviceObjLock(obj); + cap = obj->def->caps; + + while (cap) { + + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + if (STREQ(cap->data.scsi_host.wwnn, wwnn) && + STREQ(cap->data.scsi_host.wwpn, wwpn)) {
I would fetch dev and call virNodeDeviceObjUnlock(obj) here before getting to out: i.e. keep the crucial actions closer to the spot and stay generic in the exit section.
Thanks--that does make the code flow better.
+ found = 1; + goto out; + } + } + } + cap = cap->next; + } + + virNodeDeviceObjUnlock(obj); + } + +out: + if (found) { + dev = virGetNodeDevice(conn, obj->def->name); + virNodeDeviceObjUnlock(obj); + } + + return dev; +} + + static char *nodeDeviceDumpXML(virNodeDevicePtr dev, unsigned int flags ATTRIBUTE_UNUSED) { @@ -258,6 +307,310 @@ cleanup: }
+static int +nodeDeviceVportCreateDelete(virConnectPtr conn, + const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int fd = -1; + int retval = 0; + char *operation_path;
I would initilize it to NULL
Already done.
+ const char *operation_file; + char *vport_name; + size_t towrite = 0; + unsigned int written = 0; + + switch (operation) { + case VPORT_CREATE: + operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX; + break; + case VPORT_DELETE: + operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX; + break; + default: + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid vport operation (%d)"), operation); + retval = -1; + goto no_unwind; + break; + } + + if (virAsprintf(&operation_path, + "%shost%d%s", + LINUX_SYSFS_FC_HOST_PREFIX, + parent_host, + operation_file) < 0) { + + virReportOOMError(conn); + retval = -1; + goto no_unwind; + } + + VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); + + fd = open(operation_path, O_WRONLY); + + if (fd < 0) { + virReportSystemError(conn, errno, + _("Could not open '%s' for vport operation"), + operation_path); + retval = -1; + goto free_path; + } + + if (virAsprintf(&vport_name, + "%s:%s", + wwpn, + wwnn) < 0) { + + virReportOOMError(conn); + retval = -1; + goto close_fd; + } + + towrite = strlen(vport_name); + written = safewrite(fd, vport_name, towrite); + if (written != towrite) { + virReportSystemError(conn, errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed " + "(towrite: %lu written: %d)"), + vport_name, operation_path, + towrite, written); + retval = -1; + } + + VIR_FREE(vport_name); +close_fd: + close(fd); +free_path: + VIR_FREE(operation_path); +no_unwind: + VIR_DEBUG("%s", _("Vport operation complete")); + return retval; +}
and unify the 3 exit labels with if (fd >= 0) close(fd) VIR_FREE(operation_path); VIR_DEBUG(...)
if you initilize the variable on enter like fd you can have a single exit point, simpler code, less prone to breakage if the code is modified later
DanPB made the same comment; fixed.
[...]
+static int +get_parent_host(virConnectPtr conn, + virDeviceMonitorStatePtr driver, + virNodeDeviceDefPtr def, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(&driver->devs, def->parent); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Could not find parent device for '%s'"), + def->name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Device %s is not a SCSI host"), + parent->def->name); + ret = -1; + } + +out: + if (parent != NULL) { + virNodeDeviceObjUnlock(parent); + }
again I would move it before the break and replace the break to a goto out:
The unlock can be moved before the out: label and the conditional removed, so I think that's the cleanest way to do it.
I find a bit weird to have only unlocking in this function, but I assume it's just because we get a locked parent as a result of FindByName
That's correct.
+ + return ret; +} + [...] + while (dev == NULL && now - start < LINUX_NEW_DEVICE_WAIT_TIME) {
better to fully parenthesize
Huh--funny, I usually do. Thanks for pointing that out. Fixed.
+ /* We can't hold the driver lock while we wait because the + wait for devices call takes it. It's safe to drop the lock + because we're done with the driver structure at this point + anyway. We take it again when we look to see what, if + anything, was created. */ + nodeDeviceUnlock(driver); + virNodeDeviceWaitForDevices(conn); + nodeDeviceLock(driver); + + dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn); + + if (dev == NULL) { + sleep(5);
Hum, we do sleep with the lock here, and for a long time !
This point is interesting. It turns out that I didn't need to release the lock when calling wait for devices; I'm not sure where I got that idea. In any case, do I need to be holding the driver lock throughout the entire node device create operation? I think no, as the driver pointer is not going to be invalidated during the node device create operation, and I don't really care if the structure it points to changes, as I'm taking the lock again when I look to see if the device was created. If the lock *does* need to be held through the entire operation, then the 5 seconds is nothing compared to the 180 seconds that udev might take to settle out. In my devel system, it routinely takes > 30 seconds because I have so many devices and am creating so many new devices with each create call. If that's the case, then I think we need to provide an async option to the call that returns without the dev pointer and expects the caller to poll for it.
+ now = time(NULL); + if (now == (time_t)-1) { + virNodeDeviceReportError(dev->conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not get current time")); + break; + } + } + } + + return dev; +} [...] --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -53,9 +53,34 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", "80211")
+VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, + "fc_host", + "vport_ops")
#define virNodeDeviceLog(msg...) fprintf(stderr, msg)
errr, can we fix that while modifying this module, we really ought to use the normal logging internal APIs Well there are other places:
network_driver.c:#define networkLog(level, msg...) fprintf(stderr, msg) node_device_conf.c:#define virNodeDeviceLog(msg...) fprintf(stderr, msg) storage_conf.c:#define virStorageLog(msg...) fprintf(stderr, msg) storage_driver.c:#define storageLog(msg...) fprintf(stderr, msg)
I will rather do a separate PATCH to plug those...
Agreed, that should be a separate patch.
+ if ((fd = open(wwnn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out;
here fd is open but out: doesn't close it.
+ } + + close(fd);
just set back fd to -1 here and in other places where you close it and in out: add if (fd >= 0) close(fd);
Fixed, thank you for catching that.
+ if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out;
same here. Alternatively just close(fd) before goto out; in those 2 cases.
static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, union _virNodeDevCapData *d) { (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); + (void)check_fc_host(d); + (void)check_vport_capable(d); return 0; }
Hum, I find hard to justify those void casts
I'll check the return status; not sure what I was thinking there.
One thing I find missing is that we extend the capabilities but I don't see any associated documentation, or is that already covered ?
I need to add the documentation. Dave
--- src/Makefile.am | 4 +- src/node_device.c | 123 ++++++++++++++++++++++++-------- src/node_device.h | 1 - src/node_device_conf.c | 30 +++++--- src/node_device_conf.h | 6 ++- src/node_device_hal.c | 131 +---------------------------------- src/node_device_hal.h | 40 ++++++++++ src/node_device_hal_linux.c | 165 +++++++++++++++++++++++++++++++++++++++++++ src/qemu_driver.c | 2 +- src/remote_internal.c | 2 +- src/virsh.c | 56 ++++++++++++++- src/xen_unified.c | 2 +- tests/nodedevxml2xmltest.c | 2 +- 13 files changed, 384 insertions(+), 180 deletions(-) create mode 100644 src/node_device_hal.h create mode 100644 src/node_device_hal_linux.c diff --git a/src/Makefile.am b/src/Makefile.am index fd692b4..39fabce 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -188,7 +188,9 @@ NODE_DEVICE_DRIVER_SOURCES = \ node_device.c node_device.h NODE_DEVICE_DRIVER_HAL_SOURCES = \ - node_device_hal.c + node_device_hal.c \ + node_device_hal_linux.c + NODE_DEVICE_DRIVER_DEVKIT_SOURCES = \ node_device_devkit.c diff --git a/src/node_device.c b/src/node_device.c index 25d3251..41a7fd9 100644 --- a/src/node_device.c +++ b/src/node_device.c @@ -316,9 +316,8 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, { int fd = -1; int retval = 0; - char *operation_path; + char *operation_path = NULL, *vport_name = NULL; const char *operation_file; - char *vport_name; size_t towrite = 0; unsigned int written = 0; @@ -333,7 +332,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, _("Invalid vport operation (%d)"), operation); retval = -1; - goto no_unwind; + goto cleanup; break; } @@ -345,7 +344,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, virReportOOMError(conn); retval = -1; - goto no_unwind; + goto cleanup; } VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); @@ -357,7 +356,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, _("Could not open '%s' for vport operation"), operation_path); retval = -1; - goto free_path; + goto cleanup; } if (virAsprintf(&vport_name, @@ -367,7 +366,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, virReportOOMError(conn); retval = -1; - goto close_fd; + goto cleanup; } towrite = strlen(vport_name); @@ -382,12 +381,12 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, retval = -1; } +cleanup: + if (fd != -1) { + close(fd); + } VIR_FREE(vport_name); -close_fd: - close(fd); -free_path: VIR_FREE(operation_path); -no_unwind: VIR_DEBUG("%s", _("Vport operation complete")); return retval; } @@ -406,8 +405,8 @@ get_wwns(virConnectPtr conn, while (cap != NULL) { if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - *wwnn = cap->data.scsi_host.wwnn; - *wwpn = cap->data.scsi_host.wwnn; + *wwnn = strdup(cap->data.scsi_host.wwnn); + *wwpn = strdup(cap->data.scsi_host.wwpn); break; } @@ -415,13 +414,17 @@ get_wwns(virConnectPtr conn, } if (cap == NULL) { - /* XXX This error code is wrong--it results in errors of the form: - "error: invalid node device pointer in Device foo is not a fibre channel HBA" - */ - virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, - _("Device %s is not a fibre channel HBA"), - def->name); + virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("Device is not a fibre channel HBA")); + ret = -1; + } + + if (*wwnn == NULL || *wwpn == NULL) { + /* Free the other one, if allocated... */ + VIR_FREE(wwnn); + VIR_FREE(wwpn); ret = -1; + virReportOOMError(conn); } return ret; @@ -431,27 +434,30 @@ get_wwns(virConnectPtr conn, static int get_parent_host(virConnectPtr conn, virDeviceMonitorStatePtr driver, - virNodeDeviceDefPtr def, + const char *dev_name, + const char *parent_name, int *parent_host) { virNodeDeviceObjPtr parent = NULL; virNodeDevCapsDefPtr cap = NULL; int ret = 0; - parent = virNodeDeviceFindByName(&driver->devs, def->parent); + parent = virNodeDeviceFindByName(&driver->devs, parent_name); if (parent == NULL) { virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, _("Could not find parent device for '%s'"), - def->name); + dev_name); ret = -1; goto out; } cap = parent->def->caps; while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { - *parent_host = cap->data.scsi_host.host; - break; + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { + *parent_host = cap->data.scsi_host.host; + break; } cap = cap->next; @@ -459,7 +465,7 @@ get_parent_host(virConnectPtr conn, if (cap == NULL) { virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, - _("Device %s is not a SCSI host"), + _("Device %s is not capable of vport operations"), parent->def->name); ret = -1; } @@ -542,7 +548,7 @@ nodeDeviceCreateXML(virConnectPtr conn, nodeDeviceLock(driver); - def = virNodeDeviceDefParseString(conn, xmlDesc); + def = virNodeDeviceDefParseString(conn, xmlDesc, CREATE_DEVICE); if (def == NULL) { goto cleanup; } @@ -551,7 +557,11 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; } - if (get_parent_host(conn, driver, def, &parent_host) == -1) { + if (get_parent_host(conn, + driver, + def->name, + def->parent, + &parent_host) == -1) { goto cleanup; } @@ -574,14 +584,69 @@ nodeDeviceCreateXML(virConnectPtr conn, cleanup: nodeDeviceUnlock(driver); virNodeDeviceDefFree(def); + VIR_FREE(wwnn); + VIR_FREE(wwpn); return dev; } static int -nodeDeviceDestroy(virNodeDevicePtr dev ATTRIBUTE_UNUSED) +nodeDeviceDestroy(virNodeDevicePtr dev) { - return 0; + int ret = 0; + virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData; + virNodeDeviceObjPtr obj = NULL; + char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + + nodeDeviceLock(driver); + obj = virNodeDeviceFindByName(&driver->devs, dev->name); + nodeDeviceUnlock(driver); + + if (!obj) { + virNodeDeviceReportError(dev->conn, VIR_ERR_NO_NODE_DEVICE, NULL); + goto out; + } + + if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + goto out; + } + + parent_name = strdup(obj->def->parent); + + /* get_parent_host will cause the device object's lock to be + * taken, so we have to dup the parent's name and drop the lock + * before calling it. We don't need the reference to the object + * any more once we have the parent's name. */ + virNodeDeviceObjUnlock(obj); + obj = NULL; + + if (parent_name == NULL) { + virReportOOMError(dev->conn); + goto out; + } + + if (get_parent_host(dev->conn, + driver, + dev->name, + parent_name, + &parent_host) == -1) { + goto out; + } + + if (nodeDeviceVportCreateDelete(dev->conn, + parent_host, + wwpn, + wwnn, + VPORT_DELETE) == -1) { + goto out; + } + +out: + VIR_FREE(parent_name); + VIR_FREE(wwnn); + VIR_FREE(wwpn); + return ret; } diff --git a/src/node_device.h b/src/node_device.h index 59cd5f5..882ba0f 100644 --- a/src/node_device.h +++ b/src/node_device.h @@ -28,7 +28,6 @@ #include "driver.h" #include "node_device_conf.h" - #define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host" #define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" #define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/" diff --git a/src/node_device_conf.c b/src/node_device_conf.c index 6c74551..5b35b60 100644 --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -598,7 +598,8 @@ virNodeDevCapScsiHostParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, xmlNodePtr node, - union _virNodeDevCapData *data) + union _virNodeDevCapData *data, + int create) { xmlNodePtr orignode, *nodes = NULL; int ret = -1, n = 0, i; @@ -607,7 +608,8 @@ virNodeDevCapScsiHostParseXML(virConnectPtr conn, orignode = ctxt->node; ctxt->node = node; - if (virNodeDevCapsDefParseULong(conn, "number(./host[1])", ctxt, + if (create == EXISTING_DEVICE && + virNodeDevCapsDefParseULong(conn, "number(./host[1])", ctxt, &data->scsi_host.host, def, _("no SCSI host ID supplied for '%s'"), _("invalid SCSI host ID supplied for '%s'")) < 0) { @@ -948,7 +950,8 @@ static virNodeDevCapsDefPtr virNodeDevCapsDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, virNodeDeviceDefPtr def, - xmlNodePtr node) + xmlNodePtr node, + int create) { virNodeDevCapsDefPtr caps; char *tmp; @@ -992,7 +995,7 @@ virNodeDevCapsDefParseXML(virConnectPtr conn, ret = virNodeDevCapNetParseXML(conn, ctxt, def, node, &caps->data); break; case VIR_NODE_DEV_CAP_SCSI_HOST: - ret = virNodeDevCapScsiHostParseXML(conn, ctxt, def, node, &caps->data); + ret = virNodeDevCapScsiHostParseXML(conn, ctxt, def, node, &caps->data, create); break; case VIR_NODE_DEV_CAP_SCSI: ret = virNodeDevCapScsiParseXML(conn, ctxt, def, node, &caps->data); @@ -1018,7 +1021,7 @@ error: } static virNodeDeviceDefPtr -virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) +virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt, int create) { virNodeDeviceDefPtr def; virNodeDevCapsDefPtr *next_cap; @@ -1031,7 +1034,12 @@ virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) } /* Extract device name */ - def->name = virXPathString(conn, "string(./name[1])", ctxt); + if (create == EXISTING_DEVICE) { + def->name = virXPathString(conn, "string(./name[1])", ctxt); + } else { + def->name = strdup("new device"); + } + if (!def->name) { virNodeDeviceReportError(conn, VIR_ERR_NO_NAME, NULL); goto error; @@ -1051,7 +1059,7 @@ virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) next_cap = &def->caps; for (i = 0 ; i < n ; i++) { - *next_cap = virNodeDevCapsDefParseXML(conn, ctxt, def, nodes[i]); + *next_cap = virNodeDevCapsDefParseXML(conn, ctxt, def, nodes[i], create); if (!*next_cap) { VIR_FREE(nodes); goto error; @@ -1069,7 +1077,7 @@ virNodeDeviceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) } static virNodeDeviceDefPtr -virNodeDeviceDefParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) +virNodeDeviceDefParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root, int create) { xmlXPathContextPtr ctxt = NULL; virNodeDeviceDefPtr def = NULL; @@ -1087,7 +1095,7 @@ virNodeDeviceDefParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) } ctxt->node = root; - def = virNodeDeviceDefParseXML(conn, ctxt); + def = virNodeDeviceDefParseXML(conn, ctxt, create); cleanup: xmlXPathFreeContext(ctxt); @@ -1115,7 +1123,7 @@ catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) } virNodeDeviceDefPtr -virNodeDeviceDefParseString(virConnectPtr conn, const char *str) +virNodeDeviceDefParseString(virConnectPtr conn, const char *str, int create) { xmlParserCtxtPtr pctxt; xmlDocPtr xml = NULL; @@ -1146,7 +1154,7 @@ virNodeDeviceDefParseString(virConnectPtr conn, const char *str) goto cleanup; } - def = virNodeDeviceDefParseNode(conn, xml, root); + def = virNodeDeviceDefParseNode(conn, xml, root, create); cleanup: xmlFreeParserCtxt(pctxt); diff --git a/src/node_device_conf.h b/src/node_device_conf.h index 8233a91..62b4e71 100644 --- a/src/node_device_conf.h +++ b/src/node_device_conf.h @@ -28,6 +28,9 @@ #include "util.h" #include "threads.h" +#define CREATE_DEVICE 1 +#define EXISTING_DEVICE 0 + enum virNodeDevCapType { /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ VIR_NODE_DEV_CAP_SYSTEM, /* System capability */ @@ -201,7 +204,8 @@ char *virNodeDeviceDefFormat(virConnectPtr conn, const virNodeDeviceDefPtr def); virNodeDeviceDefPtr virNodeDeviceDefParseString(virConnectPtr conn, - const char *str); + const char *str, + int create); void virNodeDeviceDefFree(virNodeDeviceDefPtr def); diff --git a/src/node_device_hal.c b/src/node_device_hal.c index 5e54044..5927ba1 100644 --- a/src/node_device_hal.c +++ b/src/node_device_hal.c @@ -26,9 +26,9 @@ #include <stdio.h> #include <stdlib.h> #include <libhal.h> -#include <fcntl.h> #include "node_device_conf.h" +#include "node_device_hal.h" #include "virterror_internal.h" #include "driver.h" #include "datatypes.h" @@ -214,135 +214,6 @@ static int gather_net_cap(LibHalContext *ctx, const char *udi, } -static int check_fc_host(union _virNodeDevCapData *d) -{ - char *sysfs_path = NULL; - char *wwnn_path = NULL; - char *wwpn_path = NULL; - char *p = NULL; - int fd = -1; - char buf[64]; - struct stat st; - - VIR_DEBUG(_("Checking if host%d is an FC HBA"), d->scsi_host.host); - - if (virAsprintf(&sysfs_path, "%s/host%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { - virReportOOMError(NULL); - goto out; - } - - if (stat(sysfs_path, &st) != 0) { - /* Not an FC HBA */ - goto out; - } - - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - - if (virAsprintf(&wwnn_path, "%s/node_name", - sysfs_path) < 0) { - virReportOOMError(NULL); - goto out; - } - - if ((fd = open(wwnn_path, O_RDONLY)) < 0) { - goto out; - } - - memset(buf, 0, sizeof(buf)); - if (saferead(fd, buf, sizeof(buf)) < 0) { - goto out; - } - - close(fd); - - p = strstr(buf, "0x"); - if (p != NULL) { - p += strlen("0x"); - } else { - p = buf; - } - - d->scsi_host.wwnn = strndup(p, sizeof(buf)); - if (d->scsi_host.wwnn == NULL) { - virReportOOMError(NULL); - goto out; - } - - p = strchr(d->scsi_host.wwnn, '\n'); - if (p != NULL) { - *p = '\0'; - } - - if (virAsprintf(&wwpn_path, "%s/port_name", - sysfs_path) < 0) { - virReportOOMError(NULL); - goto out; - } - - if ((fd = open(wwpn_path, O_RDONLY)) < 0) { - goto out; - } - - memset(buf, 0, sizeof(buf)); - if (saferead(fd, buf, sizeof(buf)) < 0) { - goto out; - } - - close(fd); - - p = strstr(buf, "0x"); - if (p != NULL) { - p += strlen("0x"); - } else { - p = buf; - } - - d->scsi_host.wwpn = strndup(p, sizeof(buf)); - if (d->scsi_host.wwpn == NULL) { - virReportOOMError(NULL); - goto out; - } - - p = strchr(d->scsi_host.wwpn, '\n'); - if (p != NULL) { - *p = '\0'; - } - -out: - VIR_FREE(sysfs_path); - VIR_FREE(wwnn_path); - VIR_FREE(wwpn_path); - return 0; -} - - -static int check_vport_capable(union _virNodeDevCapData *d) -{ - char *sysfs_path = NULL; - struct stat st; - - if (virAsprintf(&sysfs_path, "%s/host%d/vport_create", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { - virReportOOMError(NULL); - goto out; - } - - if (stat(sysfs_path, &st) != 0) { - /* Not a vport capable HBA */ - goto out; - } - - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; - -out: - VIR_FREE(sysfs_path); - return 0; -} - - static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, union _virNodeDevCapData *d) { diff --git a/src/node_device_hal.h b/src/node_device_hal.h new file mode 100644 index 0000000..0b4a2ef --- /dev/null +++ b/src/node_device_hal.h @@ -0,0 +1,40 @@ +/* + * node_device_hal.h: node device enumeration - HAL-based implementation + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __VIR_NODE_DEVICE_HAL_H__ +#define __VIR_NODE_DEVICE_HAL_H__ + +#ifdef __linux__ + +#define check_fc_host(d) check_fc_host_linux(d) +int check_fc_host_linux(union _virNodeDevCapData *d); + +#define check_vport_capable(d) check_vport_capable_linux(d) +int check_vport_capable_linux(union _virNodeDevCapData *d); + +#else /* __linux__ */ + +#define check_fc_host(d) +#define check_vport_capable(d) + +#endif /* __linux__ */ + +#endif /* __VIR_NODE_DEVICE_HAL_H__ */ diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c new file mode 100644 index 0000000..37b385b --- /dev/null +++ b/src/node_device_hal_linux.c @@ -0,0 +1,165 @@ +/* + * node_device_hal_linuc.c: Linux specific code to gather device data + * not available through HAL. + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include <fcntl.h> + +#include "node_device.h" +#include "node_device_hal.h" +#include "virterror_internal.h" +#include "memory.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_NODEDEV + +#ifdef __linux__ + +int check_fc_host_linux(union _virNodeDevCapData *d) +{ + char *sysfs_path = NULL; + char *wwnn_path = NULL; + char *wwpn_path = NULL; + char *p = NULL; + int fd = -1; + char buf[64]; + struct stat st; + + VIR_DEBUG(_("Checking if host%d is an FC HBA"), d->scsi_host.host); + + if (virAsprintf(&sysfs_path, "%s/host%d", + LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host) < 0) { + virReportOOMError(NULL); + goto out; + } + + if (stat(sysfs_path, &st) != 0) { + /* Not an FC HBA */ + goto out; + } + + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + if (virAsprintf(&wwnn_path, "%s/node_name", + sysfs_path) < 0) { + virReportOOMError(NULL); + goto out; + } + + if ((fd = open(wwnn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out; + } + + close(fd); + + p = strstr(buf, "0x"); + if (p != NULL) { + p += strlen("0x"); + } else { + p = buf; + } + + d->scsi_host.wwnn = strndup(p, sizeof(buf)); + if (d->scsi_host.wwnn == NULL) { + virReportOOMError(NULL); + goto out; + } + + p = strchr(d->scsi_host.wwnn, '\n'); + if (p != NULL) { + *p = '\0'; + } + + if (virAsprintf(&wwpn_path, "%s/port_name", + sysfs_path) < 0) { + virReportOOMError(NULL); + goto out; + } + + if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + goto out; + } + + memset(buf, 0, sizeof(buf)); + if (saferead(fd, buf, sizeof(buf)) < 0) { + goto out; + } + + close(fd); + + p = strstr(buf, "0x"); + if (p != NULL) { + p += strlen("0x"); + } else { + p = buf; + } + + d->scsi_host.wwpn = strndup(p, sizeof(buf)); + if (d->scsi_host.wwpn == NULL) { + virReportOOMError(NULL); + goto out; + } + + p = strchr(d->scsi_host.wwpn, '\n'); + if (p != NULL) { + *p = '\0'; + } + +out: + VIR_FREE(sysfs_path); + VIR_FREE(wwnn_path); + VIR_FREE(wwpn_path); + return 0; +} + + +int check_vport_capable_linux(union _virNodeDevCapData *d) +{ + char *sysfs_path = NULL; + struct stat st; + + if (virAsprintf(&sysfs_path, "%s/host%d/vport_create", + LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host) < 0) { + virReportOOMError(NULL); + goto out; + } + + if (stat(sysfs_path, &st) != 0) { + /* Not a vport capable HBA */ + goto out; + } + + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + +out: + VIR_FREE(sysfs_path); + return 0; +} + +#endif /* __linux__ */ diff --git a/src/qemu_driver.c b/src/qemu_driver.c index acf08d4..bcc7518 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -5006,7 +5006,7 @@ qemudNodeDeviceGetPciInfo (virNodeDevicePtr dev, if (!xml) goto out; - def = virNodeDeviceDefParseString(dev->conn, xml); + def = virNodeDeviceDefParseString(dev->conn, xml, EXISTING_DEVICE); if (!def) goto out; diff --git a/src/remote_internal.c b/src/remote_internal.c index 24226e5..dd46e2f 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -5027,7 +5027,7 @@ remoteNodeDeviceDestroy(virNodeDevicePtr dev) args.name = dev->name; - if (call(dev->conn, priv, 0, REMOTE_PROC_NODE_DEVICE_RESET, + if (call(dev->conn, priv, 0, REMOTE_PROC_NODE_DEVICE_DESTROY, (xdrproc_t) xdr_remote_node_device_destroy_args, (char *) &args, (xdrproc_t) xdr_void, (char *) NULL) == -1) goto done; diff --git a/src/virsh.c b/src/virsh.c index 1a094eb..427d97f 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -2984,7 +2984,7 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) { virNodeDevicePtr dev = NULL; char *from; - int found; + int found = 0; int ret = TRUE; char *buffer; @@ -2992,11 +2992,13 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) return FALSE; from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (!found) { return FALSE; + } - if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { return FALSE; + } dev = virNodeDeviceCreateXML(ctl->conn, buffer, 0); free (buffer); @@ -3014,6 +3016,53 @@ cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) /* + * "nodedev-destroy" command + */ +static const vshCmdInfo info_node_device_destroy[] = { + {"help", gettext_noop("destroy a device on the node")}, + {"desc", gettext_noop("Destroy a device on the node. Note that this " + "command destroys devices on the physical host ")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_node_device_destroy[] = { + {"name", VSH_OT_DATA, VSH_OFLAG_REQ, + gettext_noop("name of the device to be destroyed")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr dev = NULL; + int ret = TRUE; + int found = 0; + char *name; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) { + return FALSE; + } + + name = vshCommandOptString(cmd, "name", &found); + if (!found) { + return FALSE; + } + + dev = virNodeDeviceLookupByName(ctl->conn, name); + + if (virNodeDeviceDestroy(dev) == 0) { + vshPrint(ctl, _("Destroyed node device '%s'\n"), name); + } else { + vshError(ctl, FALSE, _("Failed to destroy node device '%s'"), name); + ret = FALSE; + } + + virNodeDeviceFree(dev); + return ret; +} + + +/* * XML Building helper for pool-define-as and pool-create-as */ static const vshCmdOptDef opts_pool_X_as[] = { @@ -5948,6 +5997,7 @@ static const vshCmdDef commands[] = { {"nodedev-reattach", cmdNodeDeviceReAttach, opts_node_device_reattach, info_node_device_reattach}, {"nodedev-reset", cmdNodeDeviceReset, opts_node_device_reset, info_node_device_reset}, {"nodedev-create", cmdNodeDeviceCreate, opts_node_device_create, info_node_device_create}, + {"nodedev-destroy", cmdNodeDeviceDestroy, opts_node_device_destroy, info_node_device_destroy}, {"pool-autostart", cmdPoolAutostart, opts_pool_autostart, info_pool_autostart}, {"pool-build", cmdPoolBuild, opts_pool_build, info_pool_build}, diff --git a/src/xen_unified.c b/src/xen_unified.c index e708980..8da4e23 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -1439,7 +1439,7 @@ xenUnifiedNodeDeviceGetPciInfo (virNodeDevicePtr dev, if (!xml) goto out; - def = virNodeDeviceDefParseString(dev->conn, xml); + def = virNodeDeviceDefParseString(dev->conn, xml, EXISTING_DEVICE); if (!def) goto out; diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 29cdb9e..7621212 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -29,7 +29,7 @@ static int testCompareXMLToXMLFiles(const char *xml) { if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) goto fail; - if (!(dev = virNodeDeviceDefParseString(NULL, xmlData))) + if (!(dev = virNodeDeviceDefParseString(NULL, xmlData, EXISTING_DEVICE))) goto fail; if (!(actual = virNodeDeviceDefFormat(NULL, dev))) -- 1.6.0.6
participants (3)
-
Daniel Veillard -
Dave Allan -
David Allan