Re: [libvirt] [PATCH 06/15] Forward Mode 'Hostdev' qemu driver implementation

On 08/10/2012 12:24 PM, Shradha Shah wrote: Some explanation is needed in the commit log of what this is being done here. A cut-paste of the comment in the code would be a good start (any anyway, that comment can be changed since it's talking about "when there is a network with <forward mode='hostdev'>", but that "when" is "now" :-) > Signed-off-by: Shradha Shah <sshah(a)solarflare.com&gt; > --- > src/qemu/qemu_command.c | 27 +++++++++++++++++++++++++++ > 1 files changed, 27 insertions(+), 0 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 6f6c6cd..bb66364 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -24,6 +24,7 @@ > #include <config.h> > > #include "qemu_command.h" > +#include "qemu_hostdev.h" > #include "qemu_capabilities.h" > #include "qemu_bridge_filter.h" > #include "cpu/cpu.h" > @@ -5221,12 +5222,38 @@ qemuBuildCommandLine(virConnectPtr conn, > > actualType = virDomainNetGetActualType(net); > if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { > + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); > + virDomainHostdevDefPtr found; > /* type='hostdev' interfaces are handled in codepath > * for standard hostdev (NB: when there is a network > * with <forward mode='hostdev', there will need to be > * code here that adds the newly minted hostdev to the > * hostdevs array). > */ > + if (qemuAssignDeviceHostdevAlias(def, > + hostdev, Combine the above two lines. > + (def->nhostdevs-1)) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Could not assign alias to Net Hostdev")); > + goto error; > + } > + > + if (virDomainHostdevFind(def, > + hostdev, > + &found) < 0) { If the device is found already on the list, you should log an error and fail.

> + if (virDomainHostdevInsert(def, > + hostdev) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Hostdev not inserted into the array")); > + goto error; > + } > + if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, > + &hostdev, 1) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Prepare Hostdev PCI Devices failed")); > + goto error; It took me awhile to follow that trail, but I do finally understand that this is necessary (because qemuPrepareHostDevices has already been called by the time we get to here and are building the qemu commandline). > + } > + } > continue; > } > ACK with a better commit log message, fixing the comment in the code, and logging an error if the device is found already on the hostdev list. -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list