2:38 p.m.
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(a)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(a)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(a)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,
>