9:25 p.m.
I just found this review that I had typed up and not actually sent. I've
made the small changes and pushed the series in the meantime, but
figured I should send this just for archival purposes.
On 08/16/2012 11:42 AM, Shradha Shah wrote:
For a network with <forward mode='hostdev', there is a
need to
add the newly minted hostdev to the hostdevs array.
In this case we also call qemuPrepareHostDevicesas it has already been
called by the time we get to here and are building the qemu commandline
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++------
1 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f6c6cd..1470edd 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) {
- /* 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 (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net);
+ virDomainHostdevDefPtr found;
+ /* For a network with <forward mode='hostdev', there is a
need to
+ * add the newly minted hostdev to the hostdevs array.
+ */
+ if (qemuAssignDeviceHostdevAlias(def, hostdev,
+ (def->nhostdevs-1)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not assign alias to Net
Hostdev"));
qemuAssignDeviceHostdevAlias logs its own errors, so I'm removing this
log message.
+ goto error;
+ }
+
+ if (virDomainHostdevFind(def, hostdev, &found) < 0) {
+ if (virDomainHostdevInsert(def, hostdev) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Hostdev not inserted into the
array"));
virDomainHostdevInsert *doesn't* log its own errors. However, the only
reason it can fail is if we're out of memory, so I'm replacing this log
with virReportOOMError().
+ goto error;
+ }
+ if (qemuPrepareHostdevPCIDevices(driver, def->name,
def->uuid,
+ &hostdev, 1) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Prepare Hostdev PCI Devices
failed"));
This function also logs its own errors, so I'm removing this message
+ goto error;
+ }
+ }
+ else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("The Hostdev is being used by some other
device"));
How about this instead? (by this time we're guaranteed that all of those
bits of info are valid)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("PCI device %04x:%02x:%02x.%x "
"allocated from network %s is
already "
"in use by domain %s"),
hostdev->source.subsys.u.pci.domain,
hostdev->source.subsys.u.pci.bus,
hostdev->source.subsys.u.pci.slot,
hostdev->source.subsys.u.pci.function,
net->data.network.name,
def->name);
ACK with the log message changes. I'll squash them in before I push.