[libvirt] [PATCH v3 04/10] util: Management routines for scsi_host devices
Eric Farman
farman at linux.vnet.ibm.com
Mon Nov 14 20:38:53 UTC 2016
On 11/11/2016 04:44 PM, John Ferlan wrote:
> While perhaps mostly obvious - you need some sort of commit message here.
>
> On 11/08/2016 01:26 PM, Eric Farman wrote:
>> Signed-off-by: Eric Farman <farman at linux.vnet.ibm.com>
>> ---
>> po/POTFILES.in | 1 +
>> src/Makefile.am | 1 +
>> src/libvirt_private.syms | 19 +++
>> src/util/virhost.c | 299 +++++++++++++++++++++++++++++++++++++++++++++++
>> src/util/virhost.h | 72 ++++++++++++
>> src/util/virhostdev.c | 155 ++++++++++++++++++++++++
>> src/util/virhostdev.h | 16 +++
>> tests/qemuxml2argvmock.c | 9 ++
>> 8 files changed, 572 insertions(+)
>> create mode 100644 src/util/virhost.c
>> create mode 100644 src/util/virhost.h
>>
> I fear someone will equate "virhost" to host virtual functions as
> opposed to what it is. You'll note there's virhostcpu, virhostdev, and
> virhostmem - each of which are utility API's for that subsystem.
>
> So I think this needs to become 'virscsihost.{c,h}' and of course the
> API's are 'virSCSIHostDevice' prefixed instead of 'virHostDevice'.
> Alternatively, the functions could go in virscsi.c, but I do prefer the
> separation.
>
>> diff --git a/po/POTFILES.in b/po/POTFILES.in
>> index 1469240..a7cc542 100644
>> --- a/po/POTFILES.in
>> +++ b/po/POTFILES.in
>> @@ -199,6 +199,7 @@ src/util/virfirewall.c
>> src/util/virfirmware.c
>> src/util/virhash.c
>> src/util/virhook.c
>> +src/util/virhost.c
>> src/util/virhostcpu.c
>> src/util/virhostdev.c
>> src/util/virhostmem.c
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index d417b6e..404c64e 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -122,6 +122,7 @@ UTIL_SOURCES = \
>> util/virhash.c util/virhash.h \
>> util/virhashcode.c util/virhashcode.h \
>> util/virhook.c util/virhook.h \
>> + util/virhost.c util/virhost.h \
>> util/virhostcpu.c util/virhostcpu.h util/virhostcpupriv.h \
>> util/virhostdev.c util/virhostdev.h \
>> util/virhostmem.c util/virhostmem.h \
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 74dd527..ff535f9 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1675,6 +1675,23 @@ virHookInitialize;
>> virHookPresent;
>>
>>
>> +# util/virhost.h
>> +virHostDeviceFileIterate;
>> +virHostDeviceFree;
>> +virHostDeviceGetName;
>> +virHostDeviceListAdd;
>> +virHostDeviceListCount;
>> +virHostDeviceListDel;
>> +virHostDeviceListFind;
>> +virHostDeviceListFindIndex;
> This one is not used externally, so it could just be static to virhost.c
>
>> +virHostDeviceListGet;
>> +virHostDeviceListNew;
>> +virHostDeviceListSteal;
>> +virHostDeviceNew;
>> +virHostDeviceSetUsedBy;
>> +virHostOpenVhostSCSI;
>> +
>> +
>> # util/virhostdev.h
>> virHostdevFindUSBDevice;
>> virHostdevManagerGetDefault;
>> @@ -1682,10 +1699,12 @@ virHostdevPCINodeDeviceDetach;
>> virHostdevPCINodeDeviceReAttach;
>> virHostdevPCINodeDeviceReset;
>> virHostdevPrepareDomainDevices;
>> +virHostdevPrepareHostDevices;
As I alluded to in my response in patch 3, taking your s/Host/SCSIHost/
comment to heart breaks down here because
"virHostdevPrepareSCSIHostDevices" was introduced by commit 17bddc46.
So now I'm torn in how far to correlate the changes. I currently have
two (to-be-squashed) patches that does "s/virHost/virSCSIHost/" and
"s/HOST/SCSIHOST/" within the context of this series, but going farther
means qemuHostdevPrepareSCSIHostDevices calls
virHostdevPrepareHostDevices, because qemuHostdevPrepareSCSIDevices
calls virHostdevPrepareSCSIDevices, whihc may call
virHostdevPrepareSCSIHostDevices
>> virHostdevPreparePCIDevices;
>> virHostdevPrepareSCSIDevices;
>> virHostdevPrepareUSBDevices;
>> virHostdevReAttachDomainDevices;
>> +virHostdevReAttachHostDevices;
>> virHostdevReAttachPCIDevices;
>> virHostdevReAttachSCSIDevices;
>> virHostdevReAttachUSBDevices;
>> diff --git a/src/util/virhost.c b/src/util/virhost.c
>> new file mode 100644
>> index 0000000..9b5f524
>> --- /dev/null
>> +++ b/src/util/virhost.c
>> @@ -0,0 +1,299 @@
>> +/*
>> + * virhost.c: helper APIs for managing scsi_host devices
>> + *
>> + * Copyright (C) 2016 IBM Corporation
>> + *
>> + * 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, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Authors:
>> + * Eric Farman <farman at linux.vnet.ibm.com>
>> + */
>> +
>> +#include <config.h>
>> +#include <fcntl.h>
>> +
>> +#include "virhost.h"
>> +#include "virlog.h"
>> +#include "viralloc.h"
>> +#include "virerror.h"
>> +#include "virfile.h"
>> +#include "virstring.h"
>> +
>> +VIR_LOG_INIT("util.host");
>> +
>> +#define SYSFS_VHOST_SCSI_DEVICES "/sys/kernel/config/target/vhost/"
>> +#define VHOST_SCSI_DEVICE "/dev/vhost-scsi"
>> +
>> +struct _virUsedByInfo {
>> + char *drvname; /* which driver */
>> + char *domname; /* which domain */
>> +};
>> +typedef struct _virUsedByInfo *virUsedByInfoPtr;
> Hmm... seeing this makes me think the code should go in virscsi.c...
> That would seem to then allow more code reuse...
>
> However, looking at your virHostdevPrepareHostDevices changes for how
> this is implemented, I see no sharing allowed/going on. Thus, there's no
> need for a list of used_by - rather it's just a single 'used_by_drvname'
> and 'used_by_domname' in the virSCSIHostDevice structure (similar to PCI
> and USB).
>
> Unless of course you think you want to add sharing.... which I cannot
> imagine is a good idea...
No, this is a fine idea. I cannot see a point in permitting this.
>
>> +
>> +struct _virHostDevice {
> s/Host/SCSIHost/
>
>> + char *name; /* naa.<wwn> */
>> + char *path;
>> + virUsedByInfoPtr *used_by; /* driver:domain(s) using this dev */
>> + size_t n_used_by; /* how many domains are using this dev */
> Looks like the above 2 get replaced by
>
> char *used_by_drvname;
> char *used_by_domname;
>
> Also - looking at QEMU and "forward" thinking - there are options on the
> qemu command line for things like boot_tpgt, num_queues, max_sectors,
> cmd_per_lun, and bootindex... I can see the last one being asked for -
> as in can we pass one of these to the guest to allow booting from a
> specific LUN on the array...
>
>> +};
>> +
>> +struct _virHostDeviceList {
> s/Host/SCSIHost/
>
>> + virObjectLockable parent;
>> + size_t count;
>> + virHostDevicePtr *devs;
>> +};
>> +
>> +static virClassPtr virHostDeviceListClass;
> s/Host/SCSIHost/
>
> (ad nauseum ;-))
>
>> +
>> +static void
>> +virHostDeviceListDispose(void *obj)
>> +{
>> + virHostDeviceListPtr list = obj;
>> + size_t i;
>> +
>> + for (i = 0; i < list->count; i++)
>> + virHostDeviceFree(list->devs[i]);
>> +
>> + VIR_FREE(list->devs);
>> +}
>> +
>> +
> You start w/ the newer convention of 2 spaces between functions, but it
> ends here. After this there's always just 1 space between functions -
> let's try to go w/ 2.
OK.
>
>> +static int
>> +virHostOnceInit(void)
>> +{
>> + if (!(virHostDeviceListClass = virClassNew(virClassForObjectLockable(),
>> + "virHostDeviceList",
>> + sizeof(virHostDeviceList),
>> + virHostDeviceListDispose)))
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> +VIR_ONCE_GLOBAL_INIT(virHost)
>> +
>> +/* For virReportOOMError() and virReportSystemError() */
>> +#define VIR_FROM_THIS VIR_FROM_NONE
>> +
>> +int
>> +virHostOpenVhostSCSI(int *vhostfd)
>> +{
>> + if (!virFileExists(VHOST_SCSI_DEVICE))
>> + goto error;
>> +
>> + *vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR);
>> +
>> + if (*vhostfd < 0) {
>> + virReportSystemError(errno, _("Failed to open %s"), VHOST_SCSI_DEVICE);
>> + goto error;
>> + }
>> +
>> + return 0;
>> +
>> + error:
>> + VIR_FORCE_CLOSE(*vhostfd);
>> +
>> + return -1;
>> +}
>> +
>> +static void
>> +virHostDeviceUsedByInfoFree(virUsedByInfoPtr used_by)
>> +{
> I think this ends up going away since things aren't shareable.
>
>> + VIR_FREE(used_by->drvname);
>> + VIR_FREE(used_by->domname);
>> + VIR_FREE(used_by);
>> +}
>> +
>> +void
>> +virHostDeviceListDel(virHostDeviceListPtr list,
>> + virHostDevicePtr dev,
>> + const char *drvname,
>> + const char *domname)
>> +{
>> + virHostDevicePtr tmp = NULL;
>> + size_t i;
>> +
> This will need to be adjusted since (I assume) you don't want shareable
> scsi_host devices
>
>> + for (i = 0; i < dev->n_used_by; i++) {
>> + if (STREQ_NULLABLE(dev->used_by[i]->drvname, drvname) &&
>> + STREQ_NULLABLE(dev->used_by[i]->domname, domname)) {
>> + if (dev->n_used_by > 1) {
>> + virHostDeviceUsedByInfoFree(dev->used_by[i]);
>> + VIR_DELETE_ELEMENT(dev->used_by, i, dev->n_used_by);
>> + } else {
>> + tmp = virHostDeviceListSteal(list, dev);
>> + virHostDeviceFree(tmp);
>> + }
>> + break;
>> + }
>> + }
>> +}
>> +
>> +int
> s/int/static int/
>
>> +virHostDeviceListFindIndex(virHostDeviceListPtr list, virHostDevicePtr dev)
>> +{
>> + size_t i;
>> +
>> + for (i = 0; i < list->count; i++) {
>> + virHostDevicePtr other = list->devs[i];
>> + if (STREQ_NULLABLE(other->name, dev->name))
>> + return i;
>> + }
>> + return -1;
>> +}
>> +
>> +virHostDevicePtr
>> +virHostDeviceListGet(virHostDeviceListPtr list, int idx)
>> +{
>> + if (idx >= list->count || idx < 0)
>> + return NULL;
>> +
>> + return list->devs[idx];
>> +}
>> +
>> +size_t
>> +virHostDeviceListCount(virHostDeviceListPtr list)
>> +{
>> + return list->count;
>> +}
>> +
>> +virHostDevicePtr
>> +virHostDeviceListSteal(virHostDeviceListPtr list,
>> + virHostDevicePtr dev)
>> +{
>> + virHostDevicePtr ret = NULL;
>> + size_t i;
>> +
>> + for (i = 0; i < list->count; i++) {
>> + if (STREQ_NULLABLE(list->devs[i]->name, dev->name)) {
>> + ret = list->devs[i];
>> + VIR_DELETE_ELEMENT(list->devs, i, list->count);
>> + break;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +virHostDevicePtr
>> +virHostDeviceListFind(virHostDeviceListPtr list, virHostDevicePtr dev)
>> +{
>> + int idx;
>> +
>> + if ((idx = virHostDeviceListFindIndex(list, dev)) >= 0)
>> + return list->devs[idx];
>> + else
>> + return NULL;
>> +}
>> +
>> +int
>> +virHostDeviceListAdd(virHostDeviceListPtr list,
>> + virHostDevicePtr dev)
>> +{
>> + if (virHostDeviceListFind(list, dev)) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Device %s is already in use"), dev->name);
>> + return -1;
>> + }
>> + return VIR_APPEND_ELEMENT(list->devs, list->count, dev);
>> +}
>> +
>> +virHostDeviceListPtr
>> +virHostDeviceListNew(void)
>> +{
>> + virHostDeviceListPtr list;
>> +
>> + if (virHostInitialize() < 0)
>> + return NULL;
>> +
>> + if (!(list = virObjectLockableNew(virHostDeviceListClass)))
>> + return NULL;
>> +
>> + return list;
> or simply return virObjectLockableNew(virSCSIHostDeviceListClass); and
> then 'list' is not needed.
>
>> +}
>> +
>> +int
>> +virHostDeviceSetUsedBy(virHostDevicePtr dev,
>> + const char *drvname,
>> + const char *domname)
>> +{
> Without sharing, this mimics PCI/USB instead...
>
>> + virUsedByInfoPtr copy;
>> + if (VIR_ALLOC(copy) < 0)
>> + return -1;
>> + if (VIR_STRDUP(copy->drvname, drvname) < 0 ||
>> + VIR_STRDUP(copy->domname, domname) < 0)
>> + goto cleanup;
>> +
>> + if (VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy) < 0)
>> + goto cleanup;
>> +
>> + return 0;
>> +
>> + cleanup:
>> + virHostDeviceUsedByInfoFree(copy);
>> + return -1;
>> +}
>> +
>> +int
>> +virHostDeviceFileIterate(virHostDevicePtr dev,
>> + virHostDeviceFileActor actor,
>> + void *opaque)
>> +{
>> + return (actor)(dev, dev->path, opaque);
>> +}
>> +
>> +const char *
>> +virHostDeviceGetName(virHostDevicePtr dev)
>> +{
>> + return dev->name;
>> +}
>> +
>> +virHostDevicePtr
>> +virHostDeviceNew(const char *name)
>> +{
>> + virHostDevicePtr dev;
>> +
>> + if (VIR_ALLOC(dev) < 0)
>> + return NULL;
>> +
>> + if (VIR_STRDUP(dev->name, name) < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("dev->name buffer overflow: %s"),
>> + name);
>> + goto error;
>> + }
>> +
>> + if (virAsprintf(&dev->path, "%s/%s",
>> + SYSFS_VHOST_SCSI_DEVICES, name) < 0)
>> + goto cleanup;
>> +
>> + VIR_DEBUG("%s: initialized", dev->name);
>> +
>> + cleanup:
>> + return dev;
>> +
>> + error:
>> + virHostDeviceFree(dev);
>> + dev = NULL;
>> + goto cleanup;
>> +}
>> +
>> +void
>> +virHostDeviceFree(virHostDevicePtr dev)
>> +{
> size_t i;
>
>> + if (!dev)
>> + return;
>> + VIR_DEBUG("%s: freeing", dev->name);
> Would be nice if it was freeing everything in 'dev' before freeing dev...
Oops!
>
> Thus you have:
>
> VIR_FREE(dev->name);
> VIR_FREE(dev->path);
> VIR_FREE(dev->used_by_drvname);
> VIR_FREE(dev->used_by_domname);
>
>> + VIR_FREE(dev);
>> +}
>> diff --git a/src/util/virhost.h b/src/util/virhost.h
>> new file mode 100644
>> index 0000000..6d7b790
>> --- /dev/null
>> +++ b/src/util/virhost.h
>> @@ -0,0 +1,72 @@
>> +/*
>> + * virhost.h: helper APIs for managing host scsi_host devices
>> + *
>> + * Copyright (C) 2016 IBM Corporation
>> + *
>> + * 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, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Authors:
>> + * Eric Farman <farman at linux.vnet.ibm.com>
>> + */
>> +
>> +#ifndef __VIR_HOST_H__
>> +# define __VIR_HOST_H__
>> +
>> +# include "internal.h"
>> +# include "virobject.h"
>> +# include "virutil.h"
>> +
>> +typedef struct _virHostDevice virHostDevice;
>> +typedef virHostDevice *virHostDevicePtr;
>> +typedef struct _virHostDeviceAddress virHostDeviceAddress;
>> +typedef virHostDeviceAddress *virHostDeviceAddressPtr;
>> +typedef struct _virHostDeviceList virHostDeviceList;
>> +typedef virHostDeviceList *virHostDeviceListPtr;
>> +
>> +struct _virHostDeviceAddress {
>> + char *wwpn;
>> +};
> Hmmm.. this would seemingly duplicate name without the "naa.", but in
> the long run it's not even used - so why is it necessary?
>
> Then again as it turns out a "vhost-scsi-pci" or "vhost-scsi-ccw" device
> object is created. Each of those would seemingly need a controller to
> use wouldn't they?
Are you referring to a <controller> tag? Or something else?
> So the address would seem to be important, but is not
> handled.
Will check how the PCI handling is broken. Seemed okay for CCW, but
maybe I've missed something there too.
- Eric
>
>> +
>> +typedef int (*virHostDeviceFileActor)(virHostDevicePtr dev,
>> + const char *name, void *opaque);
>> +
>> +int virHostDeviceFileIterate(virHostDevicePtr dev,
>> + virHostDeviceFileActor actor,
>> + void *opaque);
>> +const char *virHostDeviceGetName(virHostDevicePtr dev);
>> +virHostDevicePtr virHostDeviceListGet(virHostDeviceListPtr list,
>> + int idx);
>> +size_t virHostDeviceListCount(virHostDeviceListPtr list);
>> +virHostDevicePtr virHostDeviceListSteal(virHostDeviceListPtr list,
>> + virHostDevicePtr dev);
>> +int virHostDeviceListFindIndex(virHostDeviceListPtr list,
>> + virHostDevicePtr dev);
> Remove the def since it's local to virhost.c
>
>> +virHostDevicePtr virHostDeviceListFind(virHostDeviceListPtr list,
>> + virHostDevicePtr dev);
>> +int virHostDeviceListAdd(virHostDeviceListPtr list,
>> + virHostDevicePtr dev);
>> +void virHostDeviceListDel(virHostDeviceListPtr list,
>> + virHostDevicePtr dev,
>> + const char *drvname,
>> + const char *domname);
>> +virHostDeviceListPtr virHostDeviceListNew(void);
>> +virHostDevicePtr virHostDeviceNew(const char *name);
>> +int virHostDeviceSetUsedBy(virHostDevicePtr dev,
>> + const char *drvname,
>> + const char *domname);
>> +void virHostDeviceFree(virHostDevicePtr dev);
>> +int virHostOpenVhostSCSI(int *vhostfd);
>> +
>> +#endif /* __VIR_HOST_H__ */
>> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
>> index 9c2262e..b92e246 100644
>> --- a/src/util/virhostdev.c
>> +++ b/src/util/virhostdev.c
>> @@ -146,6 +146,7 @@ virHostdevManagerDispose(void *obj)
>> virObjectUnref(hostdevMgr->inactivePCIHostdevs);
>> virObjectUnref(hostdevMgr->activeUSBHostdevs);
>> virObjectUnref(hostdevMgr->activeSCSIHostdevs);
>> + virObjectUnref(hostdevMgr->activeHostHostdevs);
>> VIR_FREE(hostdevMgr->stateDir);
>> }
>>
>> @@ -170,6 +171,9 @@ virHostdevManagerNew(void)
>> if (!(hostdevMgr->activeSCSIHostdevs = virSCSIDeviceListNew()))
>> goto error;
>>
>> + if (!(hostdevMgr->activeHostHostdevs = virHostDeviceListNew()))
>> + goto error;
>> +
>> if (privileged) {
>> if (VIR_STRDUP(hostdevMgr->stateDir, HOSTDEV_STATE_DIR) < 0)
>> goto error;
>> @@ -1472,6 +1476,102 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr mgr,
>> return -1;
>> }
>>
>> +int
>> +virHostdevPrepareHostDevices(virHostdevManagerPtr mgr,
>> + const char *drv_name,
>> + const char *dom_name,
>> + virDomainHostdevDefPtr *hostdevs,
>> + int nhostdevs)
>> +{
>> + size_t i, j;
>> + int count;
>> + virHostDeviceListPtr list;
>> + virHostDevicePtr tmp;
>> +
>> + if (!nhostdevs)
>> + return 0;
>> +
>> + /* To prevent situation where scsi_host device is assigned to two domains
>> + * we need to keep a list of currently assigned scsi_host devices.
>> + * This is done in several loops which cannot be joined into one big
>> + * loop. See virHostdevPreparePCIDevices()
>> + */
>> + if (!(list = virHostDeviceListNew()))
>> + goto cleanup;
>> +
>> + /* Loop 1: build temporary list */
>> + for (i = 0; i < nhostdevs; i++) {
>> + virDomainHostdevDefPtr hostdev = hostdevs[i];
>> + virDomainHostdevSubsysHostPtr hostsrc = &hostdev->source.subsys.u.host;
>> +
>> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>> + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST)
>> + continue;
>> +
>> + if (hostsrc->protocol != VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST) {
>> + continue; /* Not supported */
>> + } else {
> The else would be unnecessary - just move the virSCSIHostDevicePtr up to
> the beginning of the for loop.
>
>> + virHostDevicePtr host;
>> + if (!(host = virHostDeviceNew(hostsrc->wwpn)))
>> + goto cleanup;
>> +
>> + if (virHostDeviceListAdd(list, host) < 0) {
>> + virHostDeviceFree(host);
>> + goto cleanup;
>> + }
>> + }
>> + }
>> +
>> + /* Loop 2: Mark devices in temporary list as used by @name
>> + * and add them to driver list. However, if something goes
>> + * wrong, perform rollback.
>> + */
>> + virObjectLock(mgr->activeHostHostdevs);
>> + count = virHostDeviceListCount(list);
>> +
>> + for (i = 0; i < count; i++) {
>> + virHostDevicePtr host = virHostDeviceListGet(list, i);
> See this code doesn't do any sort of sharing - so no need for a list,
> just a single entry...
>
>> + if ((tmp = virHostDeviceListFind(mgr->activeHostHostdevs, host))) {
>> + virReportError(VIR_ERR_OPERATION_INVALID,
>> + _("SCSI_host device %s is already in use by "
>> + "another domain"),
>> + virHostDeviceGetName(tmp));
>> + goto error;
>> + } else {
>> + if (virHostDeviceSetUsedBy(host, drv_name, dom_name) < 0)
>> + goto error;
>> +
>> + VIR_DEBUG("Adding %s to activeHostHostdevs", virHostDeviceGetName(host));
>> +
>> + if (virHostDeviceListAdd(mgr->activeHostHostdevs, host) < 0)
>> + goto error;
>> + }
>> + }
>> +
>> + virObjectUnlock(mgr->activeHostHostdevs);
>> +
>> + /* Loop 3: Temporary list was successfully merged with
>> + * driver list, so steal all items to avoid freeing them
>> + * when freeing temporary list.
>> + */
>> + while (virHostDeviceListCount(list) > 0) {
>> + tmp = virHostDeviceListGet(list, 0);
>> + virHostDeviceListSteal(list, tmp);
>> + }
>> +
>> + virObjectUnref(list);
>> + return 0;
>> + error:
>> + for (j = 0; j < i; j++) {
>> + tmp = virHostDeviceListGet(list, i);
>> + virHostDeviceListSteal(mgr->activeHostHostdevs, tmp);
>> + }
>> + virObjectUnlock(mgr->activeHostHostdevs);
>> + cleanup:
>> + virObjectUnref(list);
>> + return -1;
>> +}
>> +
>> void
>> virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr,
>> const char *drv_name,
>> @@ -1604,6 +1704,61 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr mgr,
>> virObjectUnlock(mgr->activeSCSIHostdevs);
>> }
>>
>> +void
>> +virHostdevReAttachHostDevices(virHostdevManagerPtr mgr,
>> + const char *drv_name,
>> + const char *dom_name,
>> + virDomainHostdevDefPtr *hostdevs,
>> + int nhostdevs)
>> +{
>> + size_t i;
>> + virHostDevicePtr host, tmp;
>> +
>> +
>> + if (!nhostdevs)
>> + return;
>> +
>> + virObjectLock(mgr->activeHostHostdevs);
>> + for (i = 0; i < nhostdevs; i++) {
>> + virDomainHostdevDefPtr hostdev = hostdevs[i];
>> + virDomainHostdevSubsysHostPtr hostsrc = &hostdev->source.subsys.u.host;
>> +
>> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>> + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST)
>> + continue;
>> +
>> + if (hostsrc->protocol != VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST)
>> + continue; /* Not supported */
>> +
>> + if (!(host = virHostDeviceNew(hostsrc->wwpn))) {
>> + VIR_WARN("Unable to reattach SCSI_host device %s on domain %s",
>> + hostsrc->wwpn, dom_name);
>> + virObjectUnlock(mgr->activeHostHostdevs);
>> + return;
>> + }
>> +
> I assume the following changes to match PCI/USB more closely since no
> sharing is allowed.
>
>> + /* Only delete the devices which are marked as being used by @name,
>> + * because qemuProcessStart could fail half way through. */
>> +
>> + if (!(tmp = virHostDeviceListFind(mgr->activeHostHostdevs, host))) {
>> + VIR_WARN("Unable to find device %s "
>> + "in list of active SCSI_host devices",
>> + hostsrc->wwpn);
>> + virHostDeviceFree(host);
>> + virObjectUnlock(mgr->activeHostHostdevs);
>> + return;
>> + }
>> +
>> + VIR_DEBUG("Removing %s dom=%s from activeHostHostdevs",
>> + hostsrc->wwpn, dom_name);
>> +
>> + virHostDeviceListDel(mgr->activeHostHostdevs, tmp,
>> + drv_name, dom_name);
>> + virHostDeviceFree(host);
>> + }
>> + virObjectUnlock(mgr->activeHostHostdevs);
>> +}
>> +
>> int
>> virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr,
>> virPCIDevicePtr pci)
>> diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
>> index f2f51bd..19cef7e 100644
>> --- a/src/util/virhostdev.h
>> +++ b/src/util/virhostdev.h
>> @@ -30,6 +30,7 @@
>> # include "virpci.h"
>> # include "virusb.h"
>> # include "virscsi.h"
>> +# include "virhost.h"
>> # include "domain_conf.h"
>>
>> typedef enum {
>> @@ -53,6 +54,7 @@ struct _virHostdevManager {
>> virPCIDeviceListPtr inactivePCIHostdevs;
>> virUSBDeviceListPtr activeUSBHostdevs;
>> virSCSIDeviceListPtr activeSCSIHostdevs;
>> + virHostDeviceListPtr activeHostHostdevs;
> s/activeHostHostdevs/activeSCSIHostHostdevs/
>
> John
>
>> };
>>
>> virHostdevManagerPtr virHostdevManagerGetDefault(void);
>> @@ -87,6 +89,13 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr,
>> virDomainHostdevDefPtr *hostdevs,
>> int nhostdevs)
>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>> +int
>> +virHostdevPrepareHostDevices(virHostdevManagerPtr hostdev_mgr,
>> + const char *drv_name,
>> + const char *dom_name,
>> + virDomainHostdevDefPtr *hostdevs,
>> + int nhostdevs)
>> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>> void
>> virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
>> const char *drv_name,
>> @@ -109,6 +118,13 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr,
>> virDomainHostdevDefPtr *hostdevs,
>> int nhostdevs)
>> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>> +void
>> +virHostdevReAttachHostDevices(virHostdevManagerPtr hostdev_mgr,
>> + const char *drv_name,
>> + const char *dom_name,
>> + virDomainHostdevDefPtr *hostdevs,
>> + int nhostdevs)
>> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>> int
>> virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
>> virDomainHostdevDefPtr *hostdevs,
>> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
>> index 78a224b..2c85140 100644
>> --- a/tests/qemuxml2argvmock.c
>> +++ b/tests/qemuxml2argvmock.c
>> @@ -24,6 +24,7 @@
>> #include "viralloc.h"
>> #include "vircommand.h"
>> #include "vircrypto.h"
>> +#include "virhost.h"
>> #include "virmock.h"
>> #include "virnetdev.h"
>> #include "virnetdevip.h"
>> @@ -107,6 +108,14 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>> }
>>
>> int
>> +virHostOpenVhostSCSI(int *vhostfd)
>> +{
>> + *vhostfd = STDERR_FILENO + 1;
>> +
>> + return 0;
>> +}
>> +
>> +int
>> virNetDevTapCreate(char **ifname,
>> const char *tunpath ATTRIBUTE_UNUSED,
>> int *tapfd,
>>
More information about the libvir-list
mailing list