On 08/14/2012 07:44 AM, Laine Stump wrote:
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@solarflare.com> --- 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.
The device will be found on the list when using interface type=hostdev. If I log an error and fail wouldn't that mean that interface type=hostdev will always fail at this point?
+ 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list