[libvirt] [PATCH 0/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
6:42 p.m.
Version 2 of https://www.redhat.com/archives/libvir-list/2011-September/msg00795.html.
Instead of creating a new library for the TAP functions and
refactoring code to there I have only refactored the actual TAP
creation in util/bridge.c. This simplifies the patch and achieves the
same goal.
AUTHORS | 1 +
src/libvirt_bridge.syms | 2 +
src/qemu/qemu_command.c | 79 ++++++++++++++++++++++++++++++--
src/qemu/qemu_command.h | 4 ++
src/qemu/qemu_hotplug.c | 15 ++++++
src/qemu/qemu_process.c | 13 +++++
src/util/bridge.c | 116 +++++++++++++++++++++++++++++++----------------
src/util/bridge.h | 9 ++++
8 files changed, 196 insertions(+), 43 deletions(-)
6:43 p.m.
New subject: [libvirt] [PATCH 1/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
This refactors the TAP creation code out of brAddTap into a new
function brCreateTap to allow it to be used on its own. I have also
changed ifSetInterfaceMac to brSetInterfaceMac and exported it since
it is will be needed by code outside of util/bridge.c in the next
patch.
AUTHORS | 1 +
src/libvirt_bridge.syms | 2 +
src/util/bridge.c | 116 +++++++++++++++++++++++++++++++----------------
src/util/bridge.h | 9 ++++
4 files changed, 89 insertions(+), 39 deletions(-)
---
diff --git a/AUTHORS b/AUTHORS
index b3da705..391f83a 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -199,6 +199,7 @@ Patches have also been contributed by:
Dan Horák <dan(a)danny.cz>
Sage Weil <sage(a)newdream.net>
David L Stevens <dlstevens(a)us.ibm.com>
+ Tyler Coumbes <coumbes(a)gmail.com>
[....send patches to get your name here....]
diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms
index c3773bd..669830b 100644
--- a/src/libvirt_bridge.syms
+++ b/src/libvirt_bridge.syms
@@ -12,10 +12,12 @@ brAddTap;
brDeleteTap;
brDeleteBridge;
brDelInetAddress;
+brCreateTap;
brHasBridge;
brInit;
brSetEnableSTP;
brSetForwardDelay;
brSetInetNetmask;
+brSetInterfaceMac;
brSetInterfaceUp;
brShutdown;
diff --git a/src/util/bridge.c b/src/util/bridge.c
index d63b2a0..4875f52 100644
--- a/src/util/bridge.c
+++ b/src/util/bridge.c
@@ -278,7 +278,7 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED,
# endif
/**
- * ifSetInterfaceMac:
+ * brSetInterfaceMac:
* @ctl: bridge control pointer
* @ifname: interface name to set MTU for
* @macaddr: MAC address (VIR_MAC_BUFLEN in size)
@@ -288,7 +288,7 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED,
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-static int ifSetInterfaceMac(brControl *ctl, const char *ifname,
+int brSetInterfaceMac(brControl *ctl, const char *ifname,
const unsigned char *macaddr)
{
struct ifreq ifr;
@@ -478,32 +478,12 @@ brAddTap(brControl *ctl,
bool up,
int *tapfd)
{
- int fd;
- struct ifreq ifr;
-
if (!ctl || !ctl->fd || !bridge || !ifname)
return EINVAL;
- if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
- return errno;
-
- memset(&ifr, 0, sizeof(ifr));
-
- ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
+ errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd);
-# ifdef IFF_VNET_HDR
- if (vnet_hdr && brProbeVnetHdr(fd))
- ifr.ifr_flags |= IFF_VNET_HDR;
-# else
- (void) vnet_hdr;
-# endif
-
- if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
- errno = EINVAL;
- goto error;
- }
-
- if (ioctl(fd, TUNSETIFF, &ifr) < 0)
+ if(*tapfd < 0 || errno)
goto error;
/* We need to set the interface MAC before adding it
@@ -512,32 +492,22 @@ brAddTap(brControl *ctl,
* seeing the kernel allocate random MAC for the TAP
* device before we set our static MAC.
*/
- if ((errno = ifSetInterfaceMac(ctl, ifr.ifr_name, macaddr)))
+ if ((errno = brSetInterfaceMac(ctl, *ifname, macaddr)))
goto error;
/* We need to set the interface MTU before adding it
* to the bridge, because the bridge will have its
* MTU adjusted automatically when we add the new interface.
*/
- if ((errno = brSetInterfaceMtu(ctl, bridge, ifr.ifr_name)))
- goto error;
- if ((errno = brAddInterface(ctl, bridge, ifr.ifr_name)))
+ if ((errno = brSetInterfaceMtu(ctl, bridge, *ifname)))
goto error;
- if (up && ((errno = brSetInterfaceUp(ctl, ifr.ifr_name, 1))))
+ if ((errno = brAddInterface(ctl, bridge, *ifname)))
goto error;
- if (!tapfd &&
- (errno = ioctl(fd, TUNSETPERSIST, 1)))
- goto error;
- VIR_FREE(*ifname);
- if (!(*ifname = strdup(ifr.ifr_name)))
+ if (up && ((errno = brSetInterfaceUp(ctl, *ifname, 1))))
goto error;
- if (tapfd)
- *tapfd = fd;
- else
- VIR_FORCE_CLOSE(fd);
return 0;
error:
- VIR_FORCE_CLOSE(fd);
+ VIR_FORCE_CLOSE(*tapfd);
return errno;
}
@@ -803,4 +773,72 @@ cleanup:
return ret;
}
+/**
+ * brCreateTap:
+ * @ctl: bridge control pointer
+ * @ifname: the interface name
+ * @vnet_hr: whether to try enabling IFF_VNET_HDR
+ * @tapfd: file descriptor return value for the new tap device
+ *
+ * Creates a tap interface.
+ * If the @tapfd parameter is supplied, the open tap device file
+ * descriptor will be returned, otherwise the TAP device will be made
+ * persistent and closed. The caller must use brDeleteTap to remove
+ * a persistent TAP devices when it is no longer needed.
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+
+int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
+ char **ifname,
+ int vnet_hdr,
+ int *tapfd)
+{
+
+ int fd;
+ struct ifreq ifr;
+
+ if (!ifname)
+ return EINVAL;
+
+ if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
+ return errno;
+
+ memset(&ifr, 0, sizeof(ifr));
+
+ ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
+
+# ifdef IFF_VNET_HDR
+ if (vnet_hdr && brProbeVnetHdr(fd))
+ ifr.ifr_flags |= IFF_VNET_HDR;
+# else
+ (void) vnet_hdr;
+# endif
+
+ if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
+ errno = EINVAL;
+ goto error;
+ }
+
+ if (ioctl(fd, TUNSETIFF, &ifr) < 0)
+ goto error;
+
+ if (!tapfd &&
+ (errno = ioctl(fd, TUNSETPERSIST, 1)))
+ goto error;
+ VIR_FREE(*ifname);
+ if (!(*ifname = strdup(ifr.ifr_name)))
+ goto error;
+ if(tapfd)
+ *tapfd = fd;
+ else
+ VIR_FORCE_CLOSE(fd);
+ return 0;
+
+ error:
+ VIR_FORCE_CLOSE(fd);
+
+ return errno;
+}
+
#endif /* WITH_BRIDGE */
diff --git a/src/util/bridge.h b/src/util/bridge.h
index 93f0b33..c462a08 100644
--- a/src/util/bridge.h
+++ b/src/util/bridge.h
@@ -106,6 +106,15 @@ int brGetEnableSTP (brControl *ctl,
const char *bridge,
int *enable);
+int brCreateTap (brControl *ctl,
+ char **ifname,
+ int vnet_hdr,
+ int *tapfd);
+
+int brSetInterfaceMac (brControl *ctl,
+ const char *ifname,
+ const unsigned char *macaddr);
+
# endif /* WITH_BRIDGE */
#endif /* __QEMUD_BRIDGE_H__ */
10:23 p.m.
New subject: [libvirt] [PATCH 1/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
Long subject line; I trimmed it:
bridge: modify for use when sVirt is enabled with qemu
Use 'git shortlog -30' to get a feel for typical subjects.
On 10/24/2011 05:43 PM, Tyler Coumbes wrote:
This refactors the TAP creation code out of brAddTap into a new
function brCreateTap to allow it to be used on its own. I have also
changed ifSetInterfaceMac to brSetInterfaceMac and exported it since
it is will be needed by code outside of util/bridge.c in the next
patch.
AUTHORS | 1 +
src/libvirt_bridge.syms | 2 +
src/util/bridge.c | 116 +++++++++++++++++++++++++++++++----------------
src/util/bridge.h | 9 ++++
4 files changed, 89 insertions(+), 39 deletions(-)
+++ b/src/libvirt_bridge.syms
@@ -12,10 +12,12 @@ brAddTap;
brDeleteTap;
brDeleteBridge;
brDelInetAddress;
+brCreateTap;
Not sorted, but easy to fix.
-static int ifSetInterfaceMac(brControl *ctl, const char *ifname,
+int brSetInterfaceMac(brControl *ctl, const char *ifname,
const unsigned char *macaddr)
I touched up the indentation here to match.
{
struct ifreq ifr;
@@ -478,32 +478,12 @@ brAddTap(brControl *ctl,
bool up,
int *tapfd)
{
- int fd;
- struct ifreq ifr;
-
if (!ctl || !ctl->fd || !bridge || !ifname)
return EINVAL;
- if ((fd = open("/dev/net/tun", O_RDWR))< 0)
- return errno;
-
- memset(&ifr, 0, sizeof(ifr));
-
- ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
+ errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd);
Generally, we prefer returning -1 on failure, rather than a positive
errno value; but this is pre-existing.
-# ifdef IFF_VNET_HDR
- if (vnet_hdr&& brProbeVnetHdr(fd))
- ifr.ifr_flags |= IFF_VNET_HDR;
-# else
- (void) vnet_hdr;
-# endif
-
- if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
- errno = EINVAL;
- goto error;
- }
-
- if (ioctl(fd, TUNSETIFF,&ifr)< 0)
+ if(*tapfd< 0 || errno)
space after if.
}
+/**
+ * brCreateTap:
+ * @ctl: bridge control pointer
+ * @ifname: the interface name
+ * @vnet_hr: whether to try enabling IFF_VNET_HDR
+ * @tapfd: file descriptor return value for the new tap device
+ *
+ * Creates a tap interface.
+ * If the @tapfd parameter is supplied, the open tap device file
+ * descriptor will be returned, otherwise the TAP device will be made
+ * persistent and closed. The caller must use brDeleteTap to remove
+ * a persistent TAP devices when it is no longer needed.
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
So your choice of convention isn't typical, but fits the existing code.
+ */
+
+int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
No space after function names.
+ char **ifname,
+ int vnet_hdr,
+ int *tapfd)
+{
+
+ int fd;
Extra blank line.
+ struct ifreq ifr;
+
+ if (!ifname)
+ return EINVAL;
+
+ if ((fd = open("/dev/net/tun", O_RDWR))< 0)
+ return errno;
Inconsistent indentation.
+
+ memset(&ifr, 0, sizeof(ifr));
+
+ ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
+
+# ifdef IFF_VNET_HDR
+ if (vnet_hdr&& brProbeVnetHdr(fd))
+ ifr.ifr_flags |= IFF_VNET_HDR;
+# else
+ (void) vnet_hdr;
I don't like the cast to void; marking the parameter ATTRIBUTE_UNUSED is
sufficient.
But those are minor. So:
ACK. I squashed this in and pushed.
diff --git i/src/libvirt_bridge.syms w/src/libvirt_bridge.syms
index 669830b..626f6ee 100644
--- i/src/libvirt_bridge.syms
+++ w/src/libvirt_bridge.syms
@@ -9,10 +9,10 @@ brAddBridge;
brAddInetAddress;
brAddInterface;
brAddTap;
-brDeleteTap;
-brDeleteBridge;
-brDelInetAddress;
brCreateTap;
+brDelInetAddress;
+brDeleteBridge;
+brDeleteTap;
brHasBridge;
brInit;
brSetEnableSTP;
diff --git i/src/util/bridge.c w/src/util/bridge.c
index 4875f52..952f0f3 100644
--- i/src/util/bridge.c
+++ w/src/util/bridge.c
@@ -288,8 +288,9 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED,
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-int brSetInterfaceMac(brControl *ctl, const char *ifname,
- const unsigned char *macaddr)
+int
+brSetInterfaceMac(brControl *ctl, const char *ifname,
+ const unsigned char *macaddr)
{
struct ifreq ifr;
@@ -483,7 +484,7 @@ brAddTap(brControl *ctl,
errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd);
- if(*tapfd < 0 || errno)
+ if (*tapfd < 0 || errno)
goto error;
/* We need to set the interface MAC before adding it
@@ -789,12 +790,12 @@ cleanup:
* Returns 0 in case of success or an errno code in case of failure.
*/
-int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
- char **ifname,
- int vnet_hdr,
- int *tapfd)
+int
+brCreateTap(brControl *ctl ATTRIBUTE_UNUSED,
+ char **ifname,
+ int vnet_hdr ATTRIBUTE_UNUSED,
+ int *tapfd)
{
-
int fd;
struct ifreq ifr;
@@ -802,7 +803,7 @@ int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
return EINVAL;
if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
- return errno;
+ return errno;
memset(&ifr, 0, sizeof(ifr));
@@ -811,8 +812,6 @@ int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
# ifdef IFF_VNET_HDR
if (vnet_hdr && brProbeVnetHdr(fd))
ifr.ifr_flags |= IFF_VNET_HDR;
-# else
- (void) vnet_hdr;
# endif
if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:44 p.m.
New subject: [libvirt] [PATCH 2/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
This patch makes the changes to the generic ethernet interface for
QEMU. Allowing it to be used with sVirt enabled.
src/qemu/qemu_command.c | 79 ++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_command.h | 4 ++
src/qemu/qemu_hotplug.c | 15 +++++++++
src/qemu/qemu_process.c | 13 ++++++++
4 files changed, 107 insertions(+), 4 deletions(-)
---
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 30c0be6..b00920b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -404,6 +404,62 @@ qemuOpenVhostNet(virDomainDefPtr def,
}
+int qemuEthernetIfaceCreate(virDomainDefPtr def,
+ virDomainNetDefPtr net,
+ virBitmapPtr qemuCaps)
+{
+ int err;
+ int tapfd = 0;
+ int vnet_hdr = 0;
+ brControl *brctl = NULL;
+ unsigned char tapmac[VIR_MAC_BUFLEN];
+
+ if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) &&
+ net->model && STREQ(net->model, "virtio"))
+ vnet_hdr = 1;
+
+ if (!net->ifname ||
+ STRPREFIX(net->ifname, "vnet") ||
+ strchr(net->ifname, '%')) {
+ VIR_FREE(net->ifname);
+ if (!(net->ifname = strdup("vnet%d"))) {
+ virReportOOMError();
+ goto error;
+ }
+ }
+
+ if(brInit(&brctl) < 0)
+ goto error;
+
+ err = brCreateTap(brctl, &net->ifname, vnet_hdr, &tapfd);
+ virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);
+ if (tapfd < 0 || err)
+ goto error;
+
+ memcpy(tapmac, net->mac, VIR_MAC_BUFLEN);
+ /* Discourage bridge from using TAP dev MAC */
+ tapmac[0] = 0xFE;
+ err = brSetInterfaceMac(brctl, net->ifname, tapmac);
+
+ if (err)
+ goto error;
+
+ err = brSetInterfaceUp(brctl, net->ifname, 1);
+
+ if (err)
+ goto error;
+
+ brShutdown(brctl);
+
+ return tapfd;
+
+ error:
+ brShutdown(brctl);
+ VIR_FORCE_CLOSE(tapfd);
+ return -1;
+}
+
+
static int qemuDomainDeviceAliasIndex(virDomainDeviceInfoPtr info,
const char *prefix)
{
@@ -2154,14 +2210,17 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
case VIR_DOMAIN_NET_TYPE_ETHERNET:
virBufferAddLit(&buf, "tap");
- if (net->ifname) {
- virBufferAsprintf(&buf, "%cifname=%s", type_sep,
net->ifname);
- type_sep = ',';
- }
if (net->data.ethernet.script) {
+ if (net->ifname) {
+ virBufferAsprintf(&buf, "%cifname=%s", type_sep,
net->ifname);
+ type_sep = ',';
+ }
virBufferAsprintf(&buf, "%cscript=%s", type_sep,
net->data.ethernet.script);
type_sep = ',';
+ } else if(net->ifname) {
+ virBufferAsprintf(&buf, "%cfd=%s", type_sep, tapfd);
+ type_sep = ',';
}
is_tap = true;
break;
@@ -4190,6 +4249,18 @@ qemuBuildCommandLine(virConnectPtr conn,
if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
tapfd) >= sizeof(tapfd_name))
goto no_memory;
+ } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ !net->data.ethernet.script) {
+ int tapfd = qemuEthernetIfaceCreate(def, net, qemuCaps);
+ if (tapfd < 0)
+ goto error;
+
+ last_good_net = i;
+ virCommandTransferFD(cmd, tapfd);
+
+ if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+ tapfd) >= sizeof(tapfd_name))
+ goto no_memory;
}
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 00e58a2..b21eeb6 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -142,6 +142,10 @@ int qemuOpenVhostNet(virDomainDefPtr def,
virBitmapPtr qemuCaps,
int *vhostfd);
+int qemuEthernetIfaceCreate(virDomainDefPtr def,
+ virDomainNetDefPtr net,
+ virBitmapPtr qemuCaps);
+
int qemudCanonicalizeMachine(struct qemud_driver *driver,
virDomainDefPtr def);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 037f4aa..715efb2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -680,6 +680,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
iface_connected = true;
if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
goto cleanup;
+ } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ !net->data.ethernet.script)
+ {
+ if ((tapfd = qemuEthernetIfaceCreate(vm->def, net,
priv->qemuCaps)) < 0)
+ goto cleanup;
+ iface_connected = true;
}
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0)
@@ -1820,6 +1826,7 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
int vlan;
char *hostnet_name = NULL;
+ brControl *brctl = NULL;
for (i = 0 ; i < vm->def->nnets ; i++) {
virDomainNetDefPtr net = vm->def->nets[i];
@@ -1916,6 +1923,14 @@ int qemuDomainDetachNetDevice(struct
qemud_driver *driver,
}
#endif
+ if(detach->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ !detach->data.ethernet.script) {
+ if(brInit(&brctl) < 0)
+ goto cleanup;
+ brDeleteTap(brctl, detach->ifname);
+ brShutdown(brctl);
+ }
+
if ((driver->macFilter) && (detach->ifname != NULL)) {
if ((errno = networkDisallowMacOnPort(driver,
detach->ifname,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a7fe86c..b8ebf36 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3300,6 +3300,7 @@ void qemuProcessStop(struct qemud_driver *driver,
int logfile = -1;
char *timestamp;
char ebuf[1024];
+ brControl *brctl = NULL;
VIR_DEBUG("Shutting down VM '%s' pid=%d migrated=%d",
vm->def->name, vm->pid, migrated);
@@ -3421,6 +3422,18 @@ void qemuProcessStop(struct qemud_driver *driver,
networkReleaseActualDevice(net);
}
+ def = vm->def;
+ for(i=0; i < def->nnets; i++) {
+ virDomainNetDefPtr net = def->nets[i];
+ if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ !net->data.ethernet.script) {
+ if(brInit(&brctl) > 0) {
+ brDeleteTap(brctl, net->ifname);
+ brShutdown(brctl);
+ }
+ }
+ }
+
retry:
if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) {
if (ret == -EBUSY && (retries++ < 5)) {
10:25 p.m.
New subject: [libvirt] [PATCH 2/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
On 10/24/2011 05:44 PM, Tyler Coumbes wrote:
This patch makes the changes to the generic ethernet interface for
QEMU. Allowing it to be used with sVirt enabled.
src/qemu/qemu_command.c | 79 ++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_command.h | 4 ++
src/qemu/qemu_hotplug.c | 15 +++++++++
src/qemu/qemu_process.c | 13 ++++++++
4 files changed, 107 insertions(+), 4 deletions(-)
I haven't reviewed this one closely yet, but it would help if you added
a test in tests/qemuxml2argvtest.c and a new pair of files in
tests/qemuxml2argvdata/* that exercise the code to prove the new command
line options look sane.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:59 p.m.
New subject: [libvirt] [PATCH 2/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
On Thu, Oct 27, 2011 at 10:25 PM, Eric Blake <eblake(a)redhat.com> wrote:
On 10/24/2011 05:44 PM, Tyler Coumbes wrote:
>
> This patch makes the changes to the generic ethernet interface for
> QEMU. Allowing it to be used with sVirt enabled.
>
> src/qemu/qemu_command.c | 79
> ++++++++++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_command.h | 4 ++
> src/qemu/qemu_hotplug.c | 15 +++++++++
> src/qemu/qemu_process.c | 13 ++++++++
> 4 files changed, 107 insertions(+), 4 deletions(-)
I haven't reviewed this one closely yet, but it would help if you added a
test in tests/qemuxml2argvtest.c and a new pair of files in
tests/qemuxml2argvdata/* that exercise the code to prove the new command
line options look sane.
Two questions about creating a test for this code. How do I get the
actual args so I can compare them to my expected args to see what
isn't matching up? VIR_TEST_DEBUG=2 shows me what tests fail but not
any detail about why. I think it is because one of the parameters to
qemu is a file descriptor which could change for each run. Is there
some type of wildcard I can use like fd=%. I don't see any other tests
using file descriptors to compare to and am not finding anything in
any documentation I can find.
Thanks,
Tyler
11:37 a.m.
New subject: [libvirt] [PATCH 2/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
On 10/28/2011 08:59 PM, Tyler Coumbes wrote:
On Thu, Oct 27, 2011 at 10:25 PM, Eric Blake<eblake(a)redhat.com>
wrote:
> On 10/24/2011 05:44 PM, Tyler Coumbes wrote:
>>
>> This patch makes the changes to the generic ethernet interface for
>> QEMU. Allowing it to be used with sVirt enabled.
>>
>> src/qemu/qemu_command.c | 79
>> ++++++++++++++++++++++++++++++++++++++++++++--
>> src/qemu/qemu_command.h | 4 ++
>> src/qemu/qemu_hotplug.c | 15 +++++++++
>> src/qemu/qemu_process.c | 13 ++++++++
>> 4 files changed, 107 insertions(+), 4 deletions(-)
>
> I haven't reviewed this one closely yet, but it would help if you added a
> test in tests/qemuxml2argvtest.c and a new pair of files in
> tests/qemuxml2argvdata/* that exercise the code to prove the new command
> line options look sane.
>
Two questions about creating a test for this code. How do I get the
actual args so I can compare them to my expected args to see what
isn't matching up? VIR_TEST_DEBUG=2 shows me what tests fail but not
any detail about why.
VIR_TEST_DEBUG=1 shows a diff between expected and actual text; I'm not
sure why VIR_TEST_DEBUG=2 wouldn't do the same, without diving into the
test code more.
I think it is because one of the parameters to
qemu is a file descriptor which could change for each run. Is there
some type of wildcard I can use like fd=%. I don't see any other tests
using file descriptors to compare to and am not finding anything in
any documentation I can find.
tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args is an example
file that hard-codes a particular fd, and sets things up so that
qemuxml2argvtest.c passes that hard-coded fd through DO_TEST_FULL, which
in turn passes it through the migrateFd argument of qemuBuildCommandLine.
This may mean that we need to refactor things slightly to make it easier
for testing. One idea is to have normal calls to qemuBuildCommandLine
call through a function pointer that opens a TAP and returns that FD;
whereas running it from the testsuite sets that function pointer to
provide a replacement function that just returns a hard-coded number
rather than opening an fd.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:04 p.m.
New subject: [libvirt] [PATCH 2/2] V2 Modify generic ethernet interface so it will work when sVirt is enabled with qemu
On Mon, Oct 31, 2011 at 11:37 AM, Eric Blake <eblake(a)redhat.com> wrote:
On 10/28/2011 08:59 PM, Tyler Coumbes wrote:
>
> On Thu, Oct 27, 2011 at 10:25 PM, Eric Blake<eblake(a)redhat.com> wrote:
>>
>> On 10/24/2011 05:44 PM, Tyler Coumbes wrote:
>>>
>>> This patch makes the changes to the generic ethernet interface for
>>> QEMU. Allowing it to be used with sVirt enabled.
>>>
>>> src/qemu/qemu_command.c | 79
>>> ++++++++++++++++++++++++++++++++++++++++++++--
>>> src/qemu/qemu_command.h | 4 ++
>>> src/qemu/qemu_hotplug.c | 15 +++++++++
>>> src/qemu/qemu_process.c | 13 ++++++++
>>> 4 files changed, 107 insertions(+), 4 deletions(-)
>>
>> I haven't reviewed this one closely yet, but it would help if you added a
>> test in tests/qemuxml2argvtest.c and a new pair of files in
>> tests/qemuxml2argvdata/* that exercise the code to prove the new command
>> line options look sane.
>>
>
> Two questions about creating a test for this code. How do I get the
> actual args so I can compare them to my expected args to see what
> isn't matching up? VIR_TEST_DEBUG=2 shows me what tests fail but not
> any detail about why.
VIR_TEST_DEBUG=1 shows a diff between expected and actual text; I'm not sure
why VIR_TEST_DEBUG=2 wouldn't do the same, without diving into the test code
more.
> I think it is because one of the parameters to
> qemu is a file descriptor which could change for each run. Is there
> some type of wildcard I can use like fd=%. I don't see any other tests
> using file descriptors to compare to and am not finding anything in
> any documentation I can find.
tests/qemuxml2argvdata/qemuxml2argv-restore-v2-fd.args is an example file
that hard-codes a particular fd, and sets things up so that
qemuxml2argvtest.c passes that hard-coded fd through DO_TEST_FULL, which in
turn passes it through the migrateFd argument of qemuBuildCommandLine.
This may mean that we need to refactor things slightly to make it easier for
testing. One idea is to have normal calls to qemuBuildCommandLine call
through a function pointer that opens a TAP and returns that FD; whereas
running it from the testsuite sets that function pointer to provide a
replacement function that just returns a hard-coded number rather than
opening an fd.
Still not having any luck. I have to be missing something simple.
Below are the test xml and args files I have added in
tests/qemuxml2argvdata/. They are based on the qemuxml2argv-net-eth
test.
qemuxml2argv-net-eth-fd.xml
<domain type='qemu'>
<name>QEMUGuest1</name>
<uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
<memory>219100</memory>
<currentMemory>219100</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='i686' machine='pc'>hvm</type>
<boot dev='hd'/>
</os>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<target dev='hda' bus='ide'/>
<address type='drive' controller='0' bus='0'
unit='0'/>
</disk>
<controller type='ide' index='0'/>
<interface type='ethernet'>
<mac address='00:11:22:33:44:55'/>
</interface>
<memballoon model='virtio'/>
</devices>
</domain>
qemuxml2argv-net-eth-fd.args
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
-no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,\
macaddr=00:11:22:33:44:55,vlan=0 -net tap,fd=7,\
vlan=0 -serial none -parallel none -usb
At first I tried adding the following to qemuxml2argvtest.c
DO_TEST("net-eth-fd", false, NONE);
Then after your suggestion I tried
DO_TEST_FULL("net-eth-fd", "stdio", 7, false,
QEMU_CAPS_MIGRATE_QEMU_FD);
Both resulted in the same result. A failed test with no actual and
expected command line shown. Just FAILED like below.
VIR_TEST_DEBUG=1 ./qemuxml2argvtest
--snip--
83) QEMU XML-2-ARGV net-eth ... OK
84) QEMU XML-2-ARGV net-eth-fd ... FAILED
85) QEMU XML-2-ARGV net-eth-ifname ... OK
--snip--
I get the same output with VIR_TEST_DEBUG=2 as well.
No actual and expected output on the FAILED. Maybe something is going
wrong before it even gets that far? Not sure what I am missing.
Below is the actual commandline generated from a test VM running on my
development server. You can see the -netdev tap,fd=27, in the command
line.
/usr/bin/qemu-kvm -S -M pc-0.14 -cpu qemu32 -enable-kvm -m 1024 -smp
1,sockets=1,cores=1,threads=1 -name test -uuid
7fcf83f2-9c90-af39-e1f8-bdc7cadbf9c3 -nodefconfig -nodefaults -chardev
socket,id=charmonitor,path=/usr/local/var/lib/libvirt/qemu/test.monitor,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -drive
file=/vms/disks/test.img,if=none,id=drive-ide0-0-0,format=qcow2,cache=writeback
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1
-drive if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0
-netdev tap,fd=27,id=hostnet0 -device
rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:a9:37:40,bus=pci.0,addr=0x3
-chardev pty,id=charserial0 -device
isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -vga
cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
4773
days inactive
4781
days old
7 comments
2 participants
participants (2)
-
Eric Blake -
Tyler Coumbes