On Wed, Oct 23, 2013 at 04:15:57PM +0800, Chunyan Liu wrote:

2013/10/15 Daniel P. Berrange <berrange@redhat.com>

...

On Fri, Sep 13, 2013 at 11:34:34AM +0800, Chunyan Liu wrote:

...

Add hostdev passthrough common library so that it could be shared by all drivers and maintain a global hostdev state.

Signed-off-by: Chunyan Liu <cyliu@suse.com> --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 +

I'd prefer to see these changes to the XML parser be done in a separate patch - either before or after this patch.

...

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0abf80b..4d6d07b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -393,6 +393,15 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, STRNEQ(def->os.type, "hvm")) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;

+ if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + virDomainHostdevDefPtr hostdev = dev->data.hostdev; + + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) + hostdev->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; + } + return 0; }

This should be separate too.

...

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c new file mode 100644 index 0000000..d131160 --- /dev/null +++ b/src/util/virhostdev.c @@ -0,0 +1,1335 @@ +/* virhostdev.c: hostdev management + * + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. + * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * 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/>. + * + * Author: Chunyan Liu <cyliu@suse.com> + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "virhostdev.h" + +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> + +#include "viralloc.h" +#include "virstring.h" +#include "virfile.h" +#include "virerror.h" +#include "virlog.h" +#include "virpci.h" +#include "virusb.h" +#include "virscsi.h" +#include "virnetdev.h" +#include "virutil.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +#define HOSTDEV_STATE_DIR LOCALSTATEDIR "/run/libvirt/hostdevmgr" + +struct _virHostdevManager{ + char *stateDir; + + virPCIDeviceListPtr activePciHostdevs; + virPCIDeviceListPtr inactivePciHostdevs; + virUSBDeviceListPtr activeUsbHostdevs; + virSCSIDeviceListPtr activeScsiHostdevs; +}; + +static virHostdevManagerPtr hostdevMgr; + +static void +virHostdevManagerCleanup(void) +{ + if (!hostdevMgr) + return; + + virObjectUnref(hostdevMgr->activePciHostdevs); + virObjectUnref(hostdevMgr->inactivePciHostdevs); + virObjectUnref(hostdevMgr->activeUsbHostdevs); + VIR_FREE(hostdevMgr->stateDir); + + VIR_FREE(hostdevMgr); +} + +static int +virHostdevOnceInit(void) +{ + if (VIR_ALLOC(hostdevMgr) < 0) + goto error; + + if ((hostdevMgr->activePciHostdevs = virPCIDeviceListNew()) == NULL) + goto error; + + if ((hostdevMgr->activeUsbHostdevs = virUSBDeviceListNew()) == NULL) + goto error; + + if ((hostdevMgr->inactivePciHostdevs = virPCIDeviceListNew()) == NULL) + goto error; + + if ((hostdevMgr->activeScsiHostdevs = virSCSIDeviceListNew()) == NULL) + goto error; + + if (VIR_STRDUP(hostdevMgr->stateDir, HOSTDEV_STATE_DIR) < 0) + goto error;

Hmm, this is going to lead to some upgrade problems when libvirt is restarted with existing VMs present. I think we'll need some code in this initialization which looks for the old QEMU-specific path and if found, tries to move the data into the new location.

I'll do that. But IMO, finding and moving data in the initialization function is a little bit painful. The hostdevMgr->stateDir is only used to save/store net config of a SR-IOV device, the file name could be pflinkdev_ vfindex or pflinkdev (e.g, eth0, eth0_vf0, p1p2, p1p2_vf0, etc.), hard to match through some fix rule. Now that it's a upgrade requirement, and I think it only affects when an existing VM has a hostdev that stores net config in the old stateDir, we can also handle it the restore net config part. (To check whether filename is in hostdevMgr->stateDir, if not and hypervisor is qemu, try to find in old stateDir /var/run/libvirt/qemu/.) How do you think? If that's OK. I'll posting revised version.

Yeah, should be ok. Just want to minimise the amount of code to deal with back compat, whichever approach you choose. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|