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 <ss...@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.
> + 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
