[libvirt] [PATCH] virfile: safezero: fall back to writing block by block if mmap fails
mmap can fail on 32-bit systems if we're trying to zero out a lot of data.
Fall back to using block-by-block writing in that case. While we could map
smaller blocks it's unlikely that this code is used a lot and its easier to
just fall back to one of the existing methods.
Also modified the block-by-block zeroing to not allocate a megabyte of
zeroes if we're writing less than that.
Signed-off-by: Oskari Saarenmaa o...@ohmu.fi
---
src/util/virfile.c | 34 ++
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 16f8101..f662127 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1032,16 +1032,18 @@ safezero(int fd, off_t offset, off_t len)
errno = ret;
return -1;
}
+
#else
-# ifdef HAVE_MMAP
int
safezero(int fd, off_t offset, off_t len)
{
-static long pagemask = 0;
-off_t map_skip;
int r;
char *buf;
+unsigned long long remain, bytes;
+# ifdef HAVE_MMAP
+static long pagemask = 0;
+off_t map_skip;
/* align offset and length, rounding offset down and length up */
if (pagemask == 0)
@@ -1057,30 +1059,23 @@ safezero(int fd, off_t offset, off_t len)
buf = mmap(NULL, len + map_skip, PROT_READ | PROT_WRITE, MAP_SHARED,
fd, offset - map_skip);
-if (buf == MAP_FAILED)
-return -1;
+if (buf != MAP_FAILED) {
+memset(buf + map_skip, 0, len);
+munmap(buf, len + map_skip);
-memset(buf + map_skip, 0, len);
-munmap(buf, len + map_skip);
-
-return 0;
-}
-
-# else /* HAVE_MMAP */
+return 0;
+}
-int
-safezero(int fd, off_t offset, off_t len)
-{
-int r;
-char *buf;
-unsigned long long remain, bytes;
+/* fall back to writing zeroes using safewrite if mmap fails (for
+ * example because of virtual memory limits) */
+# endif /* HAVE_MMAP */
if (lseek(fd, offset, SEEK_SET) 0)
return -1;
/* Split up the write in small chunks so as not to allocate lots of RAM */
remain = len;
-bytes = 1024 * 1024;
+bytes = MAX(1024 * 1024, len);
r = VIR_ALLOC_N(buf, bytes);
if (r 0) {
@@ -1104,7 +1099,6 @@ safezero(int fd, off_t offset, off_t len)
VIR_FREE(buf);
return 0;
}
-# endif /* HAVE_MMAP */
#endif /* HAVE_POSIX_FALLOCATE */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] qemu_hotplug: Allow QoS update in qemuDomainChangeNet
The qemuDomainChangeNet() is called when 'virsh update-device' is
invoked on a NIC. Currently, we fail to update the QoS even though
we have routines for that.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_hotplug.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f06930e..275284d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1799,6 +1799,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needFilterChange = false;
bool needLinkStateChange = false;
bool needReplaceDevDef = false;
+bool needBandwidthSet = false;
int ret = -1;
if (!devslot || !(olddev = *devslot)) {
@@ -2062,8 +2063,6 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainNetGetActualDirectMode(olddev) !=
virDomainNetGetActualDirectMode(olddev) ||
!virNetDevVPortProfileEqual(virDomainNetGetActualVirtPortProfile(olddev),
virDomainNetGetActualVirtPortProfile(newdev)) ||
-!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
- virDomainNetGetActualBandwidth(newdev)) ||
!virNetDevVlanEqual(virDomainNetGetActualVlan(olddev),
virDomainNetGetActualVlan(newdev))) {
needReconnect = true;
@@ -2072,6 +2071,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
if (olddev-linkstate != newdev-linkstate)
needLinkStateChange = true;
+if (!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
+ virDomainNetGetActualBandwidth(newdev)))
+needBandwidthSet = true;
+
/* FINALLY - actually perform the required actions */
if (needReconnect) {
@@ -2081,6 +2084,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
+
+if (needBandwidthSet) {
+if (virNetDevBandwidthSet(newdev-ifname,
+ virDomainNetGetActualBandwidth(newdev),
+ false) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(cannot set bandwidth limits on %s),
+ newdev-ifname);
+goto cleanup;
+}
+needReplaceDevDef = true;
+}
+
if (needBridgeChange) {
if (qemuDomainChangeNetBridge(dom-conn, vm, olddev, newdev) 0)
goto cleanup;
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] Allow updating QoS via virDomainUpdateDeviceFlags
*** BLURB HERE *** Michal Privoznik (2): qemu_hotplug: Allow QoS update in qemuDomainChangeNet virNetDevBandwidthEqual: Make it more robust src/qemu/qemu_hotplug.c | 20 ++-- src/util/virnetdevbandwidth.c | 26 -- 2 files changed, 38 insertions(+), 8 deletions(-) -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] virNetDevBandwidthEqual: Make it more robust
So far the virNetDevBandwidthEqual() expected both -in and -out items
to be allocated for both @a and @b compared. This is not necessary true
for all our code. For instance, running 'update-device' twice over a NIC
with the very same XML results in SIGSEGV-ing in this function.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdevbandwidth.c | 26 --
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 42b0a50..17f4fa3 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -335,16 +335,30 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
return false;
/* in */
-if (a-in-average != b-in-average ||
-a-in-peak != b-in-peak ||
-a-in-burst != b-in-burst)
+if (a-in) {
+if (!b-in)
+return false;
+
+if (a-in-average != b-in-average ||
+a-in-peak != b-in-peak ||
+a-in-burst != b-in-burst)
+return false;
+} else if (b-in) {
return false;
+}
/*out*/
-if (a-out-average != b-out-average ||
-a-out-peak != b-out-peak ||
-a-out-burst != b-out-burst)
+if (a-out) {
+if (!b-out)
+return false;
+
+if (a-out-average != b-out-average ||
+a-out-peak != b-out-peak ||
+a-out-burst != b-out-burst)
+return false;
+} else if (b-out) {
return false;
+}
return true;
}
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier (IAID)
*/
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] error: server response too large
On Tue, Oct 01, 2013 at 06:57:56PM +0200, Viktor Mihajlovski wrote: On 10/01/2013 06:19 PM, Daniel P. Berrange wrote: What were you doing to get a message greater than 256KB ? This patch did not attempt to fix all possible combinations. If a API call such as virConnectListAllDomains has so much data that it requires 256KB to encode, this fix won't solve that. There is nothing we can do for API calls which have a genuine need to exceed the old size limit. I was only addressing the case of virStreamPtr data which was mistakenly hardcoded in the server to try sending 16 MB of data at once. I switched it to only try to send 256 KB of data per stream packet. OK, than it's a misunderstanding from my part (regarding your intention). The client side error message was the same as in the all-at-once case which would mean that the client somehow got past receiving the message even then and finally stumbled trying to decode the message (which is inevitable in such a case). Our S390 domains happen to be oversize due to the large number of devices (which is harder to reproduce on a PCI based system because it runs out of bus addresses too quick). So dumpxml typically raises the condition. Yep, there's nothing we can do about that scenario with old clients, since the data we're returning is inherantly too big. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-glib fails to compile with CLANG compiler
Hey, On Tue, Oct 01, 2013 at 01:05:33PM -0700, Jason Helfman wrote: Here are some build logs. The first is for amd64, and the latter is for i386. https://redports.org/~jgh/20131001181900-6317-149023/libvirt-glib-0.1.7.log https://redports.org/~jgh/20131001181900-6317-149026/libvirt-glib-0.1.7.log Most (all?) warnings are unknown warning option which should be addressed by http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=2dcdd52679ba85b7aca739efb04bdffa5a0e792b Christophe pgptrFyCsrEvW.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] qemu_hotplug: Allow QoS update in qemuDomainChangeNet
On 10/02/2013 03:34 AM, Michal Privoznik wrote:
The qemuDomainChangeNet() is called when 'virsh update-device' is
invoked on a NIC. Currently, we fail to update the QoS even though
we have routines for that.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_hotplug.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f06930e..275284d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1799,6 +1799,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needFilterChange = false;
bool needLinkStateChange = false;
bool needReplaceDevDef = false;
+bool needBandwidthSet = false;
int ret = -1;
if (!devslot || !(olddev = *devslot)) {
@@ -2062,8 +2063,6 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainNetGetActualDirectMode(olddev) !=
virDomainNetGetActualDirectMode(olddev) ||
!virNetDevVPortProfileEqual(virDomainNetGetActualVirtPortProfile(olddev),
virDomainNetGetActualVirtPortProfile(newdev)) ||
-!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
- virDomainNetGetActualBandwidth(newdev)) ||
!virNetDevVlanEqual(virDomainNetGetActualVlan(olddev),
virDomainNetGetActualVlan(newdev))) {
needReconnect = true;
@@ -2072,6 +2071,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
if (olddev-linkstate != newdev-linkstate)
needLinkStateChange = true;
+if (!virNetDevBandwidthEqual(virDomainNetGetActualBandwidth(olddev),
+ virDomainNetGetActualBandwidth(newdev)))
+needBandwidthSet = true;
+
/* FINALLY - actually perform the required actions */
if (needReconnect) {
@@ -2081,6 +2084,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
+
+if (needBandwidthSet) {
+if (virNetDevBandwidthSet(newdev-ifname,
+ virDomainNetGetActualBandwidth(newdev),
+ false) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(cannot set bandwidth limits on %s),
+ newdev-ifname);
+goto cleanup;
+}
+needReplaceDevDef = true;
+}
+
if (needBridgeChange) {
if (qemuDomainChangeNetBridge(dom-conn, vm, olddev, newdev) 0)
goto cleanup;
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] virNetDevBandwidthEqual: Make it more robust
On 10/02/2013 03:34 AM, Michal Privoznik wrote:
So far the virNetDevBandwidthEqual() expected both -in and -out items
to be allocated for both @a and @b compared. This is not necessary true
for all our code. For instance, running 'update-device' twice over a NIC
with the very same XML results in SIGSEGV-ing in this function.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virnetdevbandwidth.c | 26 --
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 42b0a50..17f4fa3 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -335,16 +335,30 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
return false;
/* in */
-if (a-in-average != b-in-average ||
-a-in-peak != b-in-peak ||
-a-in-burst != b-in-burst)
+if (a-in) {
+if (!b-in)
+return false;
+
+if (a-in-average != b-in-average ||
+a-in-peak != b-in-peak ||
+a-in-burst != b-in-burst)
+return false;
+} else if (b-in) {
return false;
+}
/*out*/
-if (a-out-average != b-out-average ||
-a-out-peak != b-out-peak ||
-a-out-burst != b-out-burst)
+if (a-out) {
+if (!b-out)
+return false;
+
+if (a-out-average != b-out-average ||
+a-out-peak != b-out-peak ||
+a-out-burst != b-out-burst)
+return false;
+} else if (b-out) {
return false;
+}
return true;
}
ACK. Could this lead to a segv prior to applying the previous patch? Or
does it only become a problem once you support bandwidth change in
qemuChangeNet?
In either case, I think this patch should be pushed upstream *before*
patch 1/2, so that we don't create a window in the history where a new
segv is introduced (just in case someone is doing a bisect and hits on
that particular revision).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3] lxc veth interfaces: fix interface name collisions
The previous veth interface naming scheme tried to find the lowest unused
index for both the parent and container veth interfaces. That's susceptible
to race conditions when multiple containers are started at the same time.
Try to pick a random unused interface id for the parent if one wasn't given
by the caller and use that as a template for the container interface name.
This should prevent races to create two uniquely named interfaces for each
container. The caller can still assign the parent interface name manually
and that name is used for in container (before the interface is moved to the
container namespace and renamed.)
Signed-off-by: Oskari Saarenmaa o...@ohmu.fi
---
My previous two patches for this issue were rejected because of concerns
with the naming scheme (in v1) or leaving fixing the race condition to the
caller (v2) and I mostly forgot about this issue after implementing a
workaround in my application, but yesterday someone else on #virt ran into
the same issue and I took another look at my patches.
The third iteration of this patch uses random identifiers and makes sure
they're not already in use, but still does not retry interface creation on
failure. I believe this is good enough as the likelihood of two containers
starting up at the same time and coming up with the same random 32-bit
identifier should be rather low.
This does change the interface names from nice low integers to random larger
integers, but I don't see that an issue. And the caller can select any
other name they like if that's not acceptable.
src/util/virnetdevveth.c | 95 ++--
1 file changed, 27 insertions(+), 68 deletions(-)
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index 039767f..9a5bc63 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -23,6 +23,7 @@
#include config.h
+#include net/if.h
#include sys/wait.h
#include virnetdevveth.h
@@ -33,119 +34,77 @@
#include virfile.h
#include virstring.h
#include virutil.h
+#include virrandom.h
#define VIR_FROM_THIS VIR_FROM_NONE
/* Functions */
/**
- * virNetDevVethGetFreeName:
- * @veth: pointer to store returned name for veth device
- * @startDev: device number to start at (x in vethx)
- *
- * Looks in /sys/class/net/ to find the first available veth device
- * name.
- *
- * Returns non-negative device number on success or -1 in case of error
- */
-static int virNetDevVethGetFreeName(char **veth, int startDev)
-{
-int devNum = startDev-1;
-char *path = NULL;
-
-VIR_DEBUG(Find free from veth%d, startDev);
-do {
-VIR_FREE(path);
-++devNum;
-if (virAsprintf(path, /sys/class/net/veth%d/, devNum) 0)
-return -1;
-VIR_DEBUG(Probe %s, path);
-} while (virFileExists(path));
-VIR_FREE(path);
-
-if (virAsprintf(veth, veth%d, devNum) 0)
-return -1;
-
-return devNum;
-}
-
-/**
* virNetDevVethCreate:
* @veth1: pointer to name for parent end of veth pair
- * @veth2: pointer to return name for container end of veth pair
+ * @veth2: pointer to name for container end of veth pair
*
* Creates a veth device pair using the ip command:
* ip link add veth1 type veth peer name veth2
- * If veth1 points to NULL on entry, it will be a valid interface on
- * return. veth2 should point to NULL on entry.
*
- * NOTE: If veth1 and veth2 names are not specified, ip will auto assign
- * names. There seems to be two problems here -
- * 1) There doesn't seem to be a way to determine the names of the
- * devices that it creates. They show up in ip link show and
- * under /sys/class/net/ however there is no guarantee that they
- * are the devices that this process just created.
- * 2) Once one of the veth devices is moved to another namespace, it
- * is no longer visible in the parent namespace. This seems to
- * confuse the name assignment causing it to fail with File exists.
- * Because of these issues, this function currently allocates names
- * prior to using the ip command, and returns any allocated names
- * to the caller.
+ * If veth1 or veth2 points to NULL on entry, they will be
+ * a valid interface on return.
*
* Returns 0 on success or -1 in case of error
*/
int virNetDevVethCreate(char** veth1, char** veth2)
{
-int rc = -1;
const char *argv[] = {
ip, link, add, NULL, type, veth, peer, name, NULL, NULL
};
-int vethDev = 0;
bool veth1_alloc = false;
bool veth2_alloc = false;
VIR_DEBUG(Host: %s guest: %s, NULLSTR(*veth1), NULLSTR(*veth2));
if (*veth1 == NULL) {
-if ((vethDev = virNetDevVethGetFreeName(veth1, vethDev)) 0)
-goto cleanup;
+size_t veth_path_max = sizeof(/sys/class/net//) + IF_NAMESIZE;
+char *veth1_path;
+
+if (VIR_ALLOC_N(*veth1, IF_NAMESIZE) 0 ||
+VIR_ALLOC_N(veth1_path,
Re: [libvirt] [PATCHv2 2/2] virNetDevBandwidthEqual: Make it more robust
On 02.10.2013 10:44, Laine Stump wrote: On 10/02/2013 03:34 AM, Michal Privoznik wrote: So far the virNetDevBandwidthEqual() expected both -in and -out items to be allocated for both @a and @b compared. This is not necessary true for all our code. For instance, running 'update-device' twice over a NIC with the very same XML results in SIGSEGV-ing in this function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virnetdevbandwidth.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) ACK. Could this lead to a segv prior to applying the previous patch? Or does it only become a problem once you support bandwidth change in qemuChangeNet? In either case, I think this patch should be pushed upstream *before* patch 1/2, so that we don't create a window in the history where a new segv is introduced (just in case someone is doing a bisect and hits on that particular revision). Good point. I've reversed the order of patches and pushed. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Attaching PCI devices to the PCIe root complex
Il 25/09/2013 10:59, Michael S. Tsirkin ha scritto: I couldn't find on PCIe spec any mention that Root Complex Integrated EndPoint must be PCIe. But, from spec 1.3.2.3: - A Root Complex Integrated Endpoint must not require I/O resources claimed through BAR(s). - A Root Complex Integrated Endpoint must not generate I/O Requests. - A Root Complex Integrated Endpoint is required to support MSI or MSI-X or both if an interrupt resource is requested. Heh PCI-SIG keeps fighting against legacy interrupts and IO. But lots of hardware happily ignores these rules. And the reason is simple: software does not enforce them. I think it's must not require, not must not have. So it's the usual rule that applies to PCIe device, i.e. that they should work even if the OS doesn't enable the I/O BARs. Then I have no idea what the I/O BAR in i915 is for, and whether the device can be used without that BAR. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ANNOUNCE: libvirt 1.0.5.6 maintenance release
On Mon, Sep 23, 2013 at 01:36:40PM -0400, Cole Robinson wrote: I basically do: - quick smoke test to make sure libvirt and virsh are working: -- stop system libvirtd -- sudo ./daemon/libvirt -- sudo ./tools/virsh list --all - make check make rpm make distcheck - cd po/ make update-po - bump configure.ac - bump rpm spec - git tag v$VER -m libvirt release $VER - commit - git push git push --tags - git clean -xdf ./autogen.sh --system make dist - upload tarball - update http://wiki.libvirt.org/page/Maintenance_Releases - send out announce mail It would be very nice if the announce mail included either a sha256 checksum for the tarball (thanks Guido for doing that in your recent release!), or a GPG key used to sign the release, see http://lwn.net/Articles/548857/ for more details about this. Christophe pgpqqev9qK0SE.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Attaching PCI devices to the PCIe root complex
On Wed, Oct 02, 2013 at 10:53:07AM +0200, Paolo Bonzini wrote: Il 25/09/2013 10:59, Michael S. Tsirkin ha scritto: I couldn't find on PCIe spec any mention that Root Complex Integrated EndPoint must be PCIe. But, from spec 1.3.2.3: - A Root Complex Integrated Endpoint must not require I/O resources claimed through BAR(s). - A Root Complex Integrated Endpoint must not generate I/O Requests. - A Root Complex Integrated Endpoint is required to support MSI or MSI-X or both if an interrupt resource is requested. Heh PCI-SIG keeps fighting against legacy interrupts and IO. But lots of hardware happily ignores these rules. And the reason is simple: software does not enforce them. I think it's must not require, not must not have. So it's the usual rule that applies to PCIe device, i.e. that they should work even if the OS doesn't enable the I/O BARs. I agree, thanks for pointing this out. Seems to still apply to the MSI rule. Then I have no idea what the I/O BAR in i915 is for, and whether the device can be used without that BAR. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier (IAID)
*/
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a
given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for
a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange berra...@redhat.com wrote:
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a
given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information
for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free
the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
The issue is, in case of leases containing ipv6 addresses, there is no
field for MAC address. Laine suggested extracting MAC address from the
cliend DUID. For example:
1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
1380692762 15172894 2001:db8:ca2:2:1::de *
00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
The last 17 chars of the client DUID represent the MAC address
[00:01:00:01:19:dd:fb:af:((52:54:00:e7:85:1e))]. But RFC3315 strictly
suggests:
Clients and servers MUST treat DUIDs as opaque values and MUST only
compare DUIDs for equality. Clients and servers MUST NOT in any
other way interpret DUIDs. Clients and servers MUST NOT restrict
DUIDs to the types defined in this document, as additional DUID types
may be defined in the future.
So, should we ignore leases containing ipv6 addresses?
--
Nehal J Wani
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/8] test: Implement readonly snapshot APIs
On 09/25/2013 03:15 PM, Cole Robinson wrote:
This is just stolen from qemu_driver.c with tweaks to fit the
test driver.
---
src/test/test_driver.c | 392
+
1 file changed, 392 insertions(+)
...
+static int
+testDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
+unsigned int flags)
+{
+virDomainObjPtr vm = NULL;
+int ret = -1;
+virDomainSnapshotObjPtr snap = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(vm = testDomObjFromSnapshot(snapshot)))
+goto cleanup;
+
Coverity complains here:
6440
(1) Event returned_pointer: Pointer snap returned by
testSnapObjFromSnapshot(vm, snapshot) is never used.
6441if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+goto cleanup;
+
+ret = (vm-current_snapshot
+ STREQ(snapshot-name, vm-current_snapshot-def-name));
+
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+
+static int
+testDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
+ unsigned int flags)
+{
+virDomainObjPtr vm = NULL;
+int ret = -1;
+virDomainSnapshotObjPtr snap = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(vm = testDomObjFromSnapshot(snapshot)))
+goto cleanup;
+
Coverity complains here too
6466
(1) Event returned_pointer: Pointer snap returned by
testSnapObjFromSnapshot(vm, snapshot) is never used.
6467if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
John
+if (!(snap = testSnapObjFromSnapshot(vm, snapshot)))
+goto cleanup;
+
+ret = 1;
+
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 02, 2013 at 09:13:59AM +0100, Daniel P. Berrange wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
Actually, I see that we already expose the IAID concept to the user
in the XML for networks
ip family=ipv6 address=2001:db8:ca2:2::1 prefix=64
dhcp
host name=paul ip=2001:db8:ca2:2:3::1 /
host id=0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66
ip=2001:db8:ca2:2:3::2 /
host id=0:3:0:1:0:16:3e:11:22:33 name=ralph
ip=2001:db8:ca2:2:3::3 /
host id=0:4:7e:7d:f0:7d:a8:bc:c5:d2:13:32:11:ed:16:ea:84:63
name=badbob ip=2001:db8:ca2:2:3::4 /
/dhcp
/ip
The 'id' value is mapping to the IAID. So it is sensible to expose this
field in the virNetworkDHCPLease struct, but as a separate field, not
as a union.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 02, 2013 at 03:41:56PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a
given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information
for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free
the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
The issue is, in case of leases containing ipv6 addresses, there is no
field for MAC address. Laine suggested extracting MAC address from the
cliend DUID. For example:
1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
1380692762 15172894 2001:db8:ca2:2:1::de *
00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
We don't want to design the API around the limitations of one particular
DHCP server implementation. If dnsmasq's leases file can't give us the
MAC addr, we still want to allow for MAC in the public struct. Have you
asked the dnsmasq developers if they're willing to add a field for the
MAC addr to the leases file for IPv6 ?
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On 10/02/2013 06:11 AM, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a
given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for
a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
The issue is, in case of leases containing ipv6 addresses, there is no
field for MAC address. Laine suggested extracting MAC address from the
cliend DUID. For example:
1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
1380692762 15172894 2001:db8:ca2:2:1::de *
00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
The last 17 chars of the client DUID represent the MAC address
[00:01:00:01:19:dd:fb:af:((52:54:00:e7:85:1e))]. But RFC3315 strictly
suggests:
Clients and servers MUST treat DUIDs as opaque values and MUST only
compare DUIDs for equality. Clients and servers MUST NOT in any
other way interpret DUIDs. Clients and servers MUST NOT restrict
DUIDs to the types defined in this document, as additional DUID types
may be defined in the future.
You aren't a client or a server (i.e. you aren't participating in the
protocol). You are a third party who is already invading the internal
state data store of one particular implementation of DHCP server -
dnsmasq (and only dnsmasq) - to try and mine some useful information. It
is a happy coincidence that the MAC address is contained in the DUID in
dnsmasq, but it *is* there, and doesn't appear to be available anywhere
else (without snooping packets on the bridge for a matching IP address).
So your choices are use that, snoop the packets, or go home. (You should
not, however, attempt to write a general purpose function intended to
derive the MAC address from the DUID of a lease for *any* DHCP
implementation - *that* is what the RFC says you must not do)
BTW, I agree with Dan that all we care about are IP address and MAC
address - IAID and DUID are just artifacts of the method used to assign
an IP address, and won't mean anything to anybody except the DHCP server
and client software.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 2, 2013 at 4:18 PM, Daniel P. Berrange berra...@redhat.com wrote:
On Wed, Oct 02, 2013 at 03:41:56PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a
given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information
for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to
free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier
(IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is
important
and deserves a separate field, or all we care about is the MAC address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
The issue is, in case of leases containing ipv6 addresses, there is no
field for MAC address. Laine suggested extracting MAC address from the
cliend DUID. For example:
1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
1380692762 15172894 2001:db8:ca2:2:1::de *
00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
We don't want to design the API around the limitations of one particular
DHCP server implementation. If dnsmasq's leases file can't give us the
MAC addr, we still want to allow for MAC in the public struct. Have you
asked the dnsmasq developers if they're willing to add a field for the
MAC addr to the leases file for IPv6 ?
I think one of them answered this question in the thread:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007538.html
One has to read all the follow-ups :)
For a tl;dr moment:
However, if you're interested in the MAC addresses of clients, the very
latest dnsmasq code can determine that in most cases. The MAC address is
not stored in the leases file, but it can be used to key configurations
to particular MAC addresses, and it's made available to the DHCP lease
external script, so an external application can use it.
I realise that adding to MAC to the leasefile for DHCPv6 clients would
make this possible, but I'm reluctant to change the leasefile format.
--
Nehal J Wani
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On Wed, Oct 02, 2013 at 04:30:16PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 4:18 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Wed, Oct 02, 2013 at 03:41:56PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange berra...@redhat.com
wrote:
On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange
berra...@redhat.com wrote:
On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for
a given
virtual network.
For DHCPv4, the information includes:
- Expirytime
- MAC Address
- IPv4 address (with type and prefix)
- Hostname (can be NULL)
- Client ID (can be NULL)
For DHCPv6, the information includes
- Expirytime
- IAID
- IPv6 address (with type and prefix)
- Hostname (can be NULL)
- Client DUID
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases
information for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to
free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association
identifier (IAID) */
+} id;
I'm not sure I like iaid - the whole point of this interface was to
return IP addresses associated with a MAC. Either the iaid is
important
and deserves a separate field, or all we care about is the MAC
address.
Not to mention that you didn't document which leg of the id union is
valid based on the type discriminator.
Agreed, we want the MAC address to be unconditionally available
here. IMHO the IAID is not something we care about exposing. That
is a impl detail of the DHCP comms protocol that is not useful
to people outside.
So in case DHCPv6 is used by the client, should we report the rest of
the lease fields and report MAC as NULL?
No, we must report the MAC. This data is useless without the MAC address
being present.
You can't even implement the virNetworkGetDHCPLeasesForMAC API without
knowing the MAC for a lease.
The issue is, in case of leases containing ipv6 addresses, there is no
field for MAC address. Laine suggested extracting MAC address from the
cliend DUID. For example:
1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
1380692762 15172894 2001:db8:ca2:2:1::de *
00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
We don't want to design the API around the limitations of one particular
DHCP server implementation. If dnsmasq's leases file can't give us the
MAC addr, we still want to allow for MAC in the public struct. Have you
asked the dnsmasq developers if they're willing to add a field for the
MAC addr to the leases file for IPv6 ?
I think one of them answered this question in the thread:
http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007538.html
One has to read all the follow-ups :)
For a tl;dr moment:
However, if you're interested in the MAC addresses of clients, the very
latest dnsmasq code can determine that in most cases. The MAC address is
not stored in the leases file, but it can be used to key configurations
to particular MAC addresses, and it's made available to the DHCP lease
external script, so an external application can use it.
So we can get the MAC addr if we use the '--dhcp-script' parameter to
make dnsmasq invoke a helper program we create, to save the lease
details we need.
Incidentally, I see our XML format is actually wrong, because it says
the 'mac' attribute is forbidden for host elements using IPv6. dnsmasq
actually allows you to specify either mac addr or IAID identifiers for
IPv6 dhcp host entries.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
Re: [libvirt] [PATCHv3] lxc veth interfaces: fix interface name collisions
On Wed, Oct 02, 2013 at 11:45:27AM +0300, Oskari Saarenmaa wrote: The previous veth interface naming scheme tried to find the lowest unused index for both the parent and container veth interfaces. That's susceptible to race conditions when multiple containers are started at the same time. Try to pick a random unused interface id for the parent if one wasn't given by the caller and use that as a template for the container interface name. This should prevent races to create two uniquely named interfaces for each container. The caller can still assign the parent interface name manually and that name is used for in container (before the interface is moved to the container namespace and renamed.) Signed-off-by: Oskari Saarenmaa o...@ohmu.fi --- My previous two patches for this issue were rejected because of concerns with the naming scheme (in v1) or leaving fixing the race condition to the caller (v2) and I mostly forgot about this issue after implementing a workaround in my application, but yesterday someone else on #virt ran into the same issue and I took another look at my patches. The third iteration of this patch uses random identifiers and makes sure they're not already in use, but still does not retry interface creation on failure. I believe this is good enough as the likelihood of two containers starting up at the same time and coming up with the same random 32-bit identifier should be rather low. This does change the interface names from nice low integers to random larger integers, but I don't see that an issue. And the caller can select any other name they like if that's not acceptable. I think having 20 digit NICs names is pretty fugly. It is possible to address the race condition by re-trying creation with new names. I will post patches todo this. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Fixes for lxc veth handling
From: Daniel P. Berrange berra...@redhat.com The primary goal of this series was to address the race condition in creation of veth device names. A few other bugs were identified along the way though fixed. Daniel P. Berrange (5): Don't set netdev offline in container cleanup Avoid reporting an error if veth device is already deleted Avoid deleting NULL veth device name Retry veth device creation on failure Use 'vnet' as prefix for veth devices src/lxc/lxc_process.c| 3 +- src/util/virnetdevveth.c | 168 ++- 2 files changed, 107 insertions(+), 64 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] Avoid deleting NULL veth device name
From: Daniel P. Berrange berra...@redhat.com
If veth device allocation has a fatal error, the veths
array may contain NULL device names. Avoid calling the
virNetDevVethDelete function on such names.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index a78784e..d07ff13 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1290,7 +1290,7 @@ cleanup:
rc = -1;
}
for (i = 0; i nveths; i++) {
-if (rc != 0)
+if (rc != 0 veths[i])
ignore_value(virNetDevVethDelete(veths[i]));
VIR_FREE(veths[i]);
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] Avoid reporting an error if veth device is already deleted
From: Daniel P. Berrange berra...@redhat.com
The kernel automatically destroys veth devices when cleaning
up the container network namepace. During normal shutdown, it
is thus likely that the attempt to run 'ip link del vethN'
will fail. If it fails, check if the device exists, and avoid
reporting an error if it has gone. This switches to use the
virCommand APIs instead of virRun too.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virnetdevveth.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index 039767f..c0d32c4 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -161,9 +161,20 @@ cleanup:
*/
int virNetDevVethDelete(const char *veth)
{
-const char *argv[] = {ip, link, del, veth, NULL};
+virCommandPtr cmd = virCommandNewArgList(ip, link, del, veth, NULL);
+int status;
-VIR_DEBUG(veth: %s, veth);
+if (virCommandRun(cmd, status) 0)
+return -1;
-return virRun(argv, NULL);
+if (status != 0) {
+if (!virNetDevExists(veth)) {
+VIR_DEBUG(Device %s already deleted (by kernel namespace
cleanup), veth);
+return 0;
+}
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to delete veth device %s), veth);
+return -1;
+}
+return 0;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] Don't set netdev offline in container cleanup
From: Daniel P. Berrange berra...@redhat.com
During container cleanup there is a race where the kernel may
have destroyed the veth device before we try to set it offline.
This causes log error messages. Given that we're about to
delete the device entirely, setting it offline is pointless.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index f92c613..a78784e 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -193,7 +193,6 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
virDomainNetDefPtr iface = vm-def-nets[i];
vport = virDomainNetGetActualVirtPortProfile(iface);
if (iface-ifname) {
-ignore_value(virNetDevSetOnline(iface-ifname, false));
if (vport
vport-virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
ignore_value(virNetDevOpenvswitchRemovePort(
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] Retry veth device creation on failure
From: Daniel P. Berrange berra...@redhat.com
The veth device creation code run in two steps, first it looks
for two free veth device names, then it runs ip link to create
the veth pair. There is an obvious race between finding free
names and creating them, when guests are started in parallel.
Rewrite the code to loop and re-try creation if it fails, to
deal with the race condition.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virnetdevveth.c | 151 +--
1 file changed, 92 insertions(+), 59 deletions(-)
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index c0d32c4..b5d618c 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -33,13 +33,26 @@
#include virfile.h
#include virstring.h
#include virutil.h
+#include virnetdev.h
#define VIR_FROM_THIS VIR_FROM_NONE
/* Functions */
+
+static int virNetDevVethExists(int devNum)
+{
+int ret;
+char *path = NULL;
+if (virAsprintf(path, /sys/class/net/veth%d/, devNum) 0)
+return -1;
+ret = virFileExists(path) ? 1 : 0;
+VIR_DEBUG(Checked dev veth%d usage: %d, devNum, ret);
+VIR_FREE(path);
+return ret;
+}
+
/**
- * virNetDevVethGetFreeName:
- * @veth: pointer to store returned name for veth device
+ * virNetDevVethGetFreeNum:
* @startDev: device number to start at (x in vethx)
*
* Looks in /sys/class/net/ to find the first available veth device
@@ -47,25 +60,23 @@
*
* Returns non-negative device number on success or -1 in case of error
*/
-static int virNetDevVethGetFreeName(char **veth, int startDev)
+static int virNetDevVethGetFreeNum(int startDev)
{
-int devNum = startDev-1;
-char *path = NULL;
+int devNum;
-VIR_DEBUG(Find free from veth%d, startDev);
-do {
-VIR_FREE(path);
-++devNum;
-if (virAsprintf(path, /sys/class/net/veth%d/, devNum) 0)
-return -1;
-VIR_DEBUG(Probe %s, path);
-} while (virFileExists(path));
-VIR_FREE(path);
+#define MAX_DEV_NUM 65536
-if (virAsprintf(veth, veth%d, devNum) 0)
-return -1;
+for (devNum = startDev ; devNum MAX_DEV_NUM ; devNum++) {
+int ret = virNetDevVethExists(devNum);
+if (ret 0)
+return -1;
+if (ret == 0)
+return devNum;
+}
-return devNum;
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(No free veth devices available));
+return -1;
}
/**
@@ -95,57 +106,79 @@ static int virNetDevVethGetFreeName(char **veth, int
startDev)
*/
int virNetDevVethCreate(char** veth1, char** veth2)
{
-int rc = -1;
-const char *argv[] = {
-ip, link, add, NULL, type, veth, peer, name, NULL, NULL
-};
-int vethDev = 0;
-bool veth1_alloc = false;
-bool veth2_alloc = false;
-
-VIR_DEBUG(Host: %s guest: %s, NULLSTR(*veth1), NULLSTR(*veth2));
-
-if (*veth1 == NULL) {
-if ((vethDev = virNetDevVethGetFreeName(veth1, vethDev)) 0)
-goto cleanup;
-VIR_DEBUG(Assigned host: %s, *veth1);
-veth1_alloc = true;
-vethDev++;
-}
-argv[3] = *veth1;
+int ret = -1;
+char *veth1auto = NULL;
+char *veth2auto = NULL;
+int vethNum = 0;
+size_t i;
+
+/*
+ * We might race with other containers, but this is reasonably
+ * unlikely, so don't do too many retries for device creation
+ */
+#define MAX_VETH_RETRIES 10
+
+for (i = 0 ; i MAX_VETH_RETRIES ; i++) {
+int status;
+if (!*veth1) {
+int veth1num;
+if ((veth1num = virNetDevVethGetFreeNum(vethNum)) 0)
+goto cleanup;
+
+if (virAsprintf(veth1auto, veth%d, veth1num) 0)
+goto cleanup;
+vethNum = veth1num + 1;
+}
+if (!*veth2) {
+int veth2num;
+if ((veth2num = virNetDevVethGetFreeNum(vethNum)) 0)
+goto cleanup;
+
+if (virAsprintf(veth2auto, veth%d, veth2num) 0)
+goto cleanup;
+vethNum = veth2num + 1;
+}
+
+virCommandPtr cmd = virCommandNew(ip);
+virCommandAddArgList(cmd, link, add,
+ *veth1 ? *veth1 : veth1auto,
+ type, veth, peer, name,
+ *veth2 ? *veth2 : veth2auto,
+ NULL);
-while (*veth2 == NULL) {
-if ((vethDev = virNetDevVethGetFreeName(veth2, vethDev)) 0) {
-if (veth1_alloc)
-VIR_FREE(*veth1);
+if (virCommandRun(cmd, status) 0)
goto cleanup;
-}
-/* Just make sure they didn't accidentally get same name */
-if (STREQ(*veth1, *veth2)) {
-vethDev++;
-VIR_FREE(*veth2);
-continue;
+if (status == 0) {
+if (veth1auto) {
+*veth1 = veth1auto;
+veth1auto = NULL;
+
[libvirt] [PATCH 5/5] Use 'vnet' as prefix for veth devices
From: Daniel P. Berrange berra...@redhat.com
The XML parser reserves 'vnet' as a prefix for automatically
generated NIC device names. Switch the veth device creation
to use this prefix, so it does not have to worry about clashes
with user specified names in the XML.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virnetdevveth.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index b5d618c..5be1539 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -43,10 +43,10 @@ static int virNetDevVethExists(int devNum)
{
int ret;
char *path = NULL;
-if (virAsprintf(path, /sys/class/net/veth%d/, devNum) 0)
+if (virAsprintf(path, /sys/class/net/vnet%d/, devNum) 0)
return -1;
ret = virFileExists(path) ? 1 : 0;
-VIR_DEBUG(Checked dev veth%d usage: %d, devNum, ret);
+VIR_DEBUG(Checked dev vnet%d usage: %d, devNum, ret);
VIR_FREE(path);
return ret;
}
@@ -125,7 +125,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
if ((veth1num = virNetDevVethGetFreeNum(vethNum)) 0)
goto cleanup;
-if (virAsprintf(veth1auto, veth%d, veth1num) 0)
+if (virAsprintf(veth1auto, vnet%d, veth1num) 0)
goto cleanup;
vethNum = veth1num + 1;
}
@@ -134,7 +134,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
if ((veth2num = virNetDevVethGetFreeNum(vethNum)) 0)
goto cleanup;
-if (virAsprintf(veth2auto, veth%d, veth2num) 0)
+if (virAsprintf(veth2auto, vnet%d, veth2num) 0)
goto cleanup;
vethNum = veth2num + 1;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] lxc veth interfaces: fix interface name collisions
On 02/10/13 14:30, Daniel P. Berrange wrote: On Wed, Oct 02, 2013 at 11:45:27AM +0300, Oskari Saarenmaa wrote: This does change the interface names from nice low integers to random larger integers, but I don't see that an issue. And the caller can select any other name they like if that's not acceptable. I think having 20 digit NICs names is pretty fugly. It is possible to address the race condition by re-trying creation with new names. I will post patches todo this. I doubt a lot people look at the interface names and care what they are (especially on servers with dozens of virtual interfaces). Also, with the new 'consistent network device naming' my desktop's interface name changed from eth0 to enp0s25 which was a bit annoying at first but makes a lot of sense with multiple devices. I think it would've also made sense to make the host interface names for containers and vms consistent, but with the limited (16 byte) size of the interface name we can't stuff the whole vm name there; a MAC address seemed like a good compromise to me and made the code shorter and simpler. Anyway, enough bikeshedding, I'm happy that the conflicts are getting fixed. Thanks, Oskari -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On 10/02/2013 04:46 AM, Daniel P. Berrange wrote: Actually, I see that we already expose the IAID concept to the user in the XML for networks ip family=ipv6 address=2001:db8:ca2:2::1 prefix=64 dhcp host name=paul ip=2001:db8:ca2:2:3::1 / host id=0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66 ip=2001:db8:ca2:2:3::2 / The 'id' value is mapping to the IAID. So it is sensible to expose this field in the virNetworkDHCPLease struct, but as a separate field, not as a union. That, and IAID is NOT an 'unsigned long long int', but an arbitrary string, and you should not be trying to parse it into a single integer. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
On 09/26/2013 02:08 AM, Nehal J Wani wrote:
+
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+long long expirytime; /* Seconds since epoch */
+union {
+char *mac; /* MAC address */
+unsigned long iaid; /* Identity association identifier (IAID) */
'unsigned long' is wrong for any new API. It is platform dependent, and
makes us have to worry about a 32-bit client talking to a 64-bit host,
and reporting OVERFLOW errors if the host sends back an answer larger
than 32 bits. If iaid is truly numeric (which I doubt), then use
'unsigned long long'. But more likely, iaid should be a char* and
treated as an opaque string, rather than trying to treat it as always
being parseable into an integer.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] LXC: Detect fs support. Mount only supported filesystems
Kept ((access(dstpath, R_OK) 0) || (!lxcCheckFSSupport(mnt-type)))
when determining support for the mount. Even if the filesystem type is
supported, there is still a chance to fail when building the dstpath
(virFileMakePath). If that call fails, starting the container will fail.
Specifically encountered this problem for securityfs, as I was unable
to mkdir /sys/kernel/security.
Signed-off-by: Bogdan Purcareata bogdan.purcare...@freescale.com
---
src/lxc/lxc_container.c | 67 +
1 file changed, 67 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 989e920..496443d 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -509,6 +509,67 @@ static int lxcContainerChildMountSort(const void *a, const
void *b)
# define MS_SLAVE(119)
#endif
+/*
+ * This function attempts to detect kernel support
+ * for a specific filesystem type. This is done by
+ * inspecting /proc/filesystems.
+ */
+static int lxcCheckFSSupport(const char *fs_type)
+{
+FILE *fp = NULL;
+int ret = -1;
+const char *fslist = /proc/filesystems;
+char *line = NULL;
+char *type;
+size_t n;
+
+/* there should be no problem mounting an entry
+ * with NULL fs type, hence NULL fs types are
+ * supported */
+if (!fs_type) {
+ ret = 1;
+ goto out;
+}
+
+VIR_DEBUG(Checking kernel support for %s in %s, fs_type, fslist);
+
+if (!(fp = fopen(fslist, r))) {
+virReportSystemError(errno,
+ _(Unable to read %s),
+ fslist);
+goto out;
+}
+
+while(getline(line, n, fp) 0) {
+ type = strstr(line, fs_type);
+
+ if (!type)
+ continue;
+
+ if (!strncmp(type, fs_type, strlen(type))) {
+ ret = 1;
+ goto cleanup;
+ }
+}
+
+if (ferror(fp)) {
+ virReportSystemError(errno,
+ _(Error reading line from %s),
+ fslist);
+goto cleanup;
+}
+
+VIR_DEBUG(No kernel support for %s, fs_type);
+
+ret = 0;
+
+cleanup:
+VIR_FREE(line);
+VIR_FORCE_FCLOSE(fp);
+out:
+return ret;
+}
+
static int lxcContainerGetSubtree(const char *prefix,
char ***mountsret,
size_t *nmountsret)
@@ -789,17 +850,23 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
for (i = 0; i ARRAY_CARDINALITY(lxcBasicMounts); i++) {
virLXCBasicMountInfo const *mnt = lxcBasicMounts[i];
const char *srcpath = NULL;
+ const char *dstpath = NULL;
VIR_DEBUG(Processing %s - %s,
mnt-src, mnt-dst);
srcpath = mnt-src;
+ dstpath = mnt-dst;
/* Skip if mount doesn't exist in source */
if ((srcpath[0] == '/')
(access(srcpath, R_OK) 0))
continue;
+ if ((access(dstpath, R_OK) 0) || /* mount is not present on host */
+ (!lxcCheckFSSupport(mnt-type))) /* no fs support in kernel */
+ continue;
+
#if WITH_SELINUX
if (STREQ(mnt-src, SELINUX_MOUNT)
(!is_selinux_enabled() || userns_enabled))
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/18] qemumonitorjsontest: Extend the test for yet another monitor commands
So far, we're unit testing some basic functions and some (so called)
simple functions (e.g. qmp_capabilities, system_powerdown). However,
there are more functions which expect simple {'return': {}} reply, but
takes more args to construct the command (for instance set_link). This
patch aims on such functions.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 97 +
1 file changed, 97 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 6f218ea..f52e970 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1037,6 +1037,69 @@ cleanup:
return ret;
}
+#define GEN_TEST_FUNC(funcName, ...)\
+static int \
+testQemuMonitorJSON ## funcName(const void *opaque) \
+{ \
+const testQemuMonitorJSONSimpleFuncDataPtr data = \
+(const testQemuMonitorJSONSimpleFuncDataPtr) opaque;\
+virDomainXMLOptionPtr xmlopt = data-xmlopt;\
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); \
+const char *reply = data-reply;\
+int ret = -1; \
+\
+if (!test) \
+return -1; \
+\
+if (!reply) \
+reply = {\return\:{}}; \
+\
+if (qemuMonitorTestAddItem(test, data-cmd, reply) 0) \
+goto cleanup; \
+\
+if (funcName(qemuMonitorTestGetMonitor(test), __VA_ARGS__) 0) \
+goto cleanup; \
+\
+ret = 0;\
+cleanup:\
+qemuMonitorTestFree(test); \
+return ret; \
+}
+
+GEN_TEST_FUNC(qemuMonitorJSONSetLink, vnet0,
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN)
+GEN_TEST_FUNC(qemuMonitorJSONBlockResize, vda, 123456)
+GEN_TEST_FUNC(qemuMonitorJSONSetVNCPassword, secret_password)
+GEN_TEST_FUNC(qemuMonitorJSONSetPassword, spice, secret_password,
disconnect)
+GEN_TEST_FUNC(qemuMonitorJSONExpirePassword, spice, 123456)
+GEN_TEST_FUNC(qemuMonitorJSONSetBalloon, 1024)
+GEN_TEST_FUNC(qemuMonitorJSONSetCPU, 1, true)
+GEN_TEST_FUNC(qemuMonitorJSONEjectMedia, hdc, true)
+GEN_TEST_FUNC(qemuMonitorJSONChangeMedia, hdc, /foo/bar, NULL)
+GEN_TEST_FUNC(qemuMonitorJSONSaveVirtualMemory, 0, 1024, /foo/bar)
+GEN_TEST_FUNC(qemuMonitorJSONSavePhysicalMemory, 0, 1024, /foo/bar)
+GEN_TEST_FUNC(qemuMonitorJSONSetMigrationSpeed, 1024)
+GEN_TEST_FUNC(qemuMonitorJSONSetMigrationDowntime, 1)
+GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND
+ QEMU_MONITOR_MIGRATE_NON_SHARED_DISK
+ QEMU_MONITOR_MIGRATE_NON_SHARED_INC, tcp:localhost:12345)
+GEN_TEST_FUNC(qemuMonitorJSONDump, dummy_protocol)
+GEN_TEST_FUNC(qemuMonitorJSONGraphicsRelocate, VIR_DOMAIN_GRAPHICS_TYPE_SPICE,
+ localhost, 12345, 12346, NULL)
+GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, some_dummy_netdevstr)
+GEN_TEST_FUNC(qemuMonitorJSONRemoveNetdev, net0)
+GEN_TEST_FUNC(qemuMonitorJSONDelDevice, ide0)
+GEN_TEST_FUNC(qemuMonitorJSONAddDevice, some_dummy_devicestr)
+GEN_TEST_FUNC(qemuMonitorJSONSetDrivePassphrase, vda, secret_passhprase)
+GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, vdb, /foo/bar, NULL, 1024,
+ VIR_DOMAIN_BLOCK_REBASE_SHALLOW
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
+GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, vdb, /foo/bar1, /foo/bar2,
1024)
+GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, vdb, NULL, NULL)
+GEN_TEST_FUNC(qemuMonitorJSONScreendump, /foo/bar)
+GEN_TEST_FUNC(qemuMonitorJSONOpenGraphics, spice, spicefd, false)
+GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, localhost, 12345)
+GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, vda, true)
+GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, serial1)
+
static int
mymain(void)
{
@@ -1065,6 +1128,11 @@ mymain(void)
if (virtTestRun(# FNC, 1, testQemuMonitorJSONSimpleFunc, simpleFunc) 0)
\
ret = -1
+#define DO_TEST_GEN(name, ...) \
+
[libvirt] [PATCH 04/18] qemuMonitorTestFree: Join worker thread unconditionally
The thread needs to be joined no matter if it was still running when qemuMonitorTestFree is called or not. The worker is thread spawned in qemuMonitorTestNew() and has to be joined. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitortestutils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index cd43c7b..4989183 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -354,8 +354,7 @@ qemuMonitorTestFree(qemuMonitorTestPtr test) virObjectUnref(test-vm); -if (test-running) -virThreadJoin(test-thread); +virThreadJoin(test-thread); if (timer != -1) virEventRemoveTimeout(timer); -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/18] qemuMonitorJSONSendKey: Avoid double free
After successful @cmd construction the memory where @keys points to is part of @cmd. Avoid double freeing it. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_monitor_json.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index fc57c00..cd6cb72 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3452,6 +3452,9 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, if (!cmd) goto cleanup; +/* @keys is part of @cmd now. Avoid double free */ +keys = NULL; + if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0) goto cleanup; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/18] qemumonitorjsontest: Test qemuMonitorJSONGetBlockInfo
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 143
1 file changed, 143 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index ba2de45..7a851bd 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1250,6 +1250,148 @@ cleanup:
}
static int
+testHashEqualQemuDomainDiskInfo(const void *value1, const void *value2)
+{
+const struct qemuDomainDiskInfo *info1 = value1, *info2 = value2;
+
+return memcmp(info1, info2, sizeof(*info1));
+}
+
+static int
+testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+virHashTablePtr blockDevices = NULL, expectedBlockDevices = NULL;
+struct qemuDomainDiskInfo *info;
+
+if (!test)
+return -1;
+
+if (!(blockDevices = virHashCreate(32, (virHashDataFree) free)) ||
+!(expectedBlockDevices = virHashCreate(32, (virHashDataFree) (free
+goto cleanup;
+
+if (VIR_ALLOC(info) 0)
+goto cleanup;
+
+if (virHashAddEntry(expectedBlockDevices, virtio-disk0, info) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Unable to create expectedBlockDevices hash table);
+goto cleanup;
+}
+
+if (VIR_ALLOC(info) 0)
+goto cleanup;
+
+if (virHashAddEntry(expectedBlockDevices, virtio-disk1, info) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Unable to create expectedBlockDevices hash table);
+goto cleanup;
+}
+
+if (VIR_ALLOC(info) 0)
+goto cleanup;
+
+info-locked = true;
+info-removable = true;
+if (virHashAddEntry(expectedBlockDevices, ide0-1-0, info) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Unable to create expectedBlockDevices hash table);
+goto cleanup;
+}
+
+if (qemuMonitorTestAddItem(test, query-block,
+ {
+ \return\: [
+ {
+ \io-status\: \ok\,
+ \device\:
\drive-virtio-disk0\,
+ \locked\: false,
+ \removable\: false,
+ \inserted\: {
+ \iops_rd\: 0,
+ \iops_wr\: 0,
+ \ro\: false,
+ \backing_file_depth\: 0,
+ \drv\: \qcow2\,
+ \iops\: 0,
+ \bps_wr\: 0,
+ \encrypted\: false,
+ \bps\: 0,
+ \bps_rd\: 0,
+ \file\:
\/home/zippy/work/tmp/gentoo.qcow2\,
+ \encryption_key_missing\:
false
+ },
+ \type\: \unknown\
+ },
+ {
+ \io-status\: \ok\,
+ \device\:
\drive-virtio-disk1\,
+ \locked\: false,
+ \removable\: false,
+ \inserted\: {
+ \iops_rd\: 0,
+ \iops_wr\: 0,
+ \ro\: false,
+ \backing_file_depth\: 0,
+ \drv\: \raw\,
+ \iops\: 0,
+ \bps_wr\: 0,
+ \encrypted\: false,
+ \bps\: 0,
+ \bps_rd\: 0,
+ \file\:
\/home/zippy/test.bin\,
+ \encryption_key_missing\:
false
+ },
+ \type\: \unknown\
+ },
+ {
+ \io-status\: \ok\,
+ \device\:
[libvirt] [PATCH 01/18] qemuMonitorJSONGetVirtType: Fix error message
When querying for kvm, we try to find 'enabled' field. Hence the error
message should report we haven't found 'enabled' and not 'running'
(which is not even in the reply). Probably a typo or copy-paste error.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_monitor_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2d84161..fc57c00 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1300,7 +1300,7 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
if (virJSONValueObjectGetBoolean(data, enabled, val) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(info kvm reply missing 'running' field));
+ _(info kvm reply missing 'enabled' field));
ret = -1;
goto cleanup;
}
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/18] qemumonitorjsontest: Test qemuMonitorJSONGetVirtType
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 6eb26a1..587b66f 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1177,6 +1177,43 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetVirtType(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+int virtType;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-kvm,
+ {
+ \return\: {
+ \enabled\: true,
+ \present\: true
+ },
+ \id\: \libvirt-8\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetVirtType(qemuMonitorTestGetMonitor(test), virtType)
0)
+goto cleanup;
+
+if (virtType != VIR_DOMAIN_VIRT_KVM) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Unexpected virt type: %d, virtType);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1260,6 +1297,7 @@ mymain(void)
DO_TEST_GEN(qemuMonitorJSONNBDServerAdd);
DO_TEST_GEN(qemuMonitorJSONDetachCharDev);
DO_TEST(qemuMonitorJSONGetCPUInfo);
+DO_TEST(qemuMonitorJSONGetVirtType);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationStatus
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 50 +
1 file changed, 50 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 52df486..576288b 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1639,6 +1639,55 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetMigrationStatus(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+qemuMonitorMigrationStatus status, expectedStatus;
+
+if (!test)
+return -1;
+
+memset(expectedStatus, 0, sizeof(expectedStatus));
+
+expectedStatus.status = QEMU_MONITOR_MIGRATION_STATUS_ACTIVE;
+expectedStatus.total_time = 47;
+expectedStatus.ram_total = 1611038720;
+expectedStatus.ram_remaining = 1605013504;
+expectedStatus.ram_transferred = 3625548;
+
+if (qemuMonitorTestAddItem(test, query-migrate,
+ {
+ \return\: {
+ \status\: \active\,
+ \total-time\: 47,
+ \ram\: {
+ \total\: 1611038720,
+ \remaining\: 1605013504,
+ \transferred\: 3625548
+ }
+ },
+ \id\: \libvirt-13\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetMigrationStatus(qemuMonitorTestGetMonitor(test),
status) 0)
+goto cleanup;
+
+if (memcmp(status, expectedStatus, sizeof(status)) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Invalid migration status);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1727,6 +1776,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetBlockInfo);
DO_TEST(qemuMonitorJSONGetBlockStatsInfo);
DO_TEST(qemuMonitorJSONGetMigrationCacheSize);
+DO_TEST(qemuMonitorJSONGetMigrationStatus);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCapability
And so do qemuMonitorJSONSetMigrationCapability.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 46 +
1 file changed, 46 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 5636fb6..6e6cfaf 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1899,6 +1899,51 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetMigrationCapability(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+int cap;
+const char *reply =
+{
+\return\: [
+{
+\state\: false,
+\capability\: \xbzrle\
+}
+],
+\id\: \libvirt-22\
+};
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-migrate-capabilities, reply) 0
||
+qemuMonitorTestAddItem(test, migrate-set-capabilities,
+ {\return\:{}}) 0)
+goto cleanup;
+
+cap =
qemuMonitorJSONGetMigrationCapability(qemuMonitorTestGetMonitor(test),
+
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE);
+if (cap != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Unexpected capability: %d, expecting 1,
+ cap);
+goto cleanup;
+}
+
+if (qemuMonitorJSONSetMigrationCapability(qemuMonitorTestGetMonitor(test),
+
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE) 0)
+goto cleanup;
+
+ret = 0;
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1993,6 +2038,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONSendKey);
DO_TEST(qemuMonitorJSONSetBlockIoThrottle);
DO_TEST(qemuMonitorJSONGetTargetArch);
+DO_TEST(qemuMonitorJSONGetMigrationCapability);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/18] qemumonitorjsontest: Test qemuMonitorJSONSetBlockIoThrottle
And so do qemuMonitorJSONGetBlockIoThrottle.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 185 +++-
1 file changed, 115 insertions(+), 70 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2c5c452..9e9dbcc 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -40,6 +40,77 @@ struct _testQemuMonitorJSONSimpleFuncData {
const char *reply;
};
+const char *queryBlockReply =
+{
+\return\: [
+{
+\io-status\: \ok\,
+\device\: \drive-virtio-disk0\,
+\locked\: false,
+\removable\: false,
+\inserted\: {
+\iops_rd\: 5,
+\iops_wr\: 6,
+\ro\: false,
+\backing_file_depth\: 0,
+\drv\: \qcow2\,
+\iops\: 4,
+\bps_wr\: 3,
+\encrypted\: false,
+\bps\: 1,
+\bps_rd\: 2,
+\file\: \/home/zippy/work/tmp/gentoo.qcow2\,
+\encryption_key_missing\: false
+},
+\type\: \unknown\
+},
+{
+\io-status\: \ok\,
+\device\: \drive-virtio-disk1\,
+\locked\: false,
+\removable\: false,
+\inserted\: {
+\iops_rd\: 0,
+\iops_wr\: 0,
+\ro\: false,
+\backing_file_depth\: 0,
+\drv\: \raw\,
+\iops\: 0,
+\bps_wr\: 0,
+\encrypted\: false,
+\bps\: 0,
+\bps_rd\: 0,
+\file\: \/home/zippy/test.bin\,
+\encryption_key_missing\: false
+},
+\type\: \unknown\
+},
+{
+\io-status\: \ok\,
+\device\: \drive-ide0-1-0\,
+\locked\: true,
+\removable\: true,
+\inserted\: {
+\iops_rd\: 0,
+\iops_wr\: 0,
+\ro\: true,
+\backing_file_depth\: 0,
+\drv\: \raw\,
+\iops\: 0,
+\bps_wr\: 0,
+\encrypted\: false,
+\bps\: 0,
+\bps_rd\: 0,
+\file\:
\/home/zippy/tmp/install-amd64-minimal-20121210.iso\,
+\encryption_key_missing\: false
+},
+\tray_open\: false,
+\type\: \unknown\
+}
+],
+\id\: \libvirt-10\
+};
+
static int
testQemuMonitorJSONGetStatus(const void *data)
{
@@ -1302,76 +1373,7 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const
void *data)
goto cleanup;
}
-if (qemuMonitorTestAddItem(test, query-block,
- {
- \return\: [
- {
- \io-status\: \ok\,
- \device\:
\drive-virtio-disk0\,
- \locked\: false,
- \removable\: false,
- \inserted\: {
- \iops_rd\: 0,
- \iops_wr\: 0,
- \ro\: false,
- \backing_file_depth\: 0,
- \drv\: \qcow2\,
- \iops\: 0,
- \bps_wr\: 0,
- \encrypted\: false,
- \bps\: 0,
- \bps_rd\: 0,
- \file\:
\/home/zippy/work/tmp/gentoo.qcow2\,
- \encryption_key_missing\:
false
- },
- \type\: \unknown\
- },
- {
- \io-status\: \ok\,
- \device\:
\drive-virtio-disk1\,
- \locked\: false,
- \removable\: false,
- \inserted\: {
- \iops_rd\: 0,
- \iops_wr\: 0,
- \ro\: false,
- \backing_file_depth\: 0,
- \drv\: \raw\,
- \iops\: 0,
-
[libvirt] [PATCH 06/18] qemumonitorjsontest: Test qemuMonitorJSONGetCPUInfo
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 77 +
1 file changed, 77 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index f52e970..6eb26a1 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1100,6 +1100,82 @@ GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart,
localhost, 12345)
GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, vda, true)
GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, serial1)
+
+static int
+testQemuMonitorJSONqemuMonitorJSONGetCPUInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+pid_t *cpupids = NULL;
+pid_t expected_cpupids[] = {17622, 17623, 17624, 17625};
+int ncpupids;
+size_t i;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-cpus,
+ {
+ \return\: [
+ {
+ \current\: true,
+ \CPU\: 0,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17622
+ },
+ {
+ \current\: false,
+ \CPU\: 1,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17623
+ },
+ {
+ \current\: false,
+ \CPU\: 2,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17624
+ },
+ {
+ \current\: false,
+ \CPU\: 3,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17625
+ }
+ ],
+ \id\: \libvirt-7\
+ }) 0)
+goto cleanup;
+
+ncpupids = qemuMonitorJSONGetCPUInfo(qemuMonitorTestGetMonitor(test),
cpupids);
+
+if (ncpupids != 4) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Expecting ncpupids = 4 but got %d, ncpupids);
+goto cleanup;
+}
+
+for (i = 0; i ncpupids; i++) {
+if (cpupids[i] != expected_cpupids[i]) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Expecting cpupids[%zu] = %d but got %d,
+ i, expected_cpupids[i], cpupids[i]);
+goto cleanup;
+}
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(cpupids);
+qemuMonitorTestFree(test);
+return ret;
+}
+
static int
mymain(void)
{
@@ -1183,6 +1259,7 @@ mymain(void)
DO_TEST_GEN(qemuMonitorJSONNBDServerStart);
DO_TEST_GEN(qemuMonitorJSONNBDServerAdd);
DO_TEST_GEN(qemuMonitorJSONDetachCharDev);
+DO_TEST(qemuMonitorJSONGetCPUInfo);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/18] qemumonitorjsontest: Test qemuMonitorJSONGetPtyPaths
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 67 +
1 file changed, 67 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index b6adb35..14c64fd 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1727,6 +1727,72 @@ cleanup:
}
static int
+testHashEqualString(const void *value1, const void *value2)
+{
+return strcmp(value1, value2);
+}
+
+static int
+testQemuMonitorJSONqemuMonitorJSONGetPtyPaths(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+virHashTablePtr paths = NULL, expectedPaths = NULL;
+
+if (!test)
+return -1;
+
+if (!(paths = virHashCreate(32, (virHashDataFree) free)) ||
+!(expectedPaths = virHashCreate(32, NULL)))
+goto cleanup;
+
+if (virHashAddEntry(expectedPaths, charserial1, (void *) /dev/pts/21)
0 ||
+virHashAddEntry(expectedPaths, charserial0, (void *) /dev/pts/20)
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Unable to create expectedPaths hash table);
+goto cleanup;
+}
+
+if (qemuMonitorTestAddItem(test, query-chardev,
+ {
+ \return\: [
+ {
+ \filename\: \pty:/dev/pts/21\,
+ \label\: \charserial1\
+ },
+ {
+ \filename\: \pty:/dev/pts/20\,
+ \label\: \charserial0\
+ },
+ {
+ \filename\:
\unix:/var/lib/libvirt/qemu/gentoo.monitor,server\,
+ \label\: \charmonitor\
+ }
+ ],
+ \id\: \libvirt-15\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetPtyPaths(qemuMonitorTestGetMonitor(test),
+ paths) 0)
+goto cleanup;
+
+if (!virHashEqual(paths, expectedPaths, testHashEqualString)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ Hashtable is different to the expected one);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+virHashFree(paths);
+virHashFree(expectedPaths);
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1817,6 +1883,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetMigrationCacheSize);
DO_TEST(qemuMonitorJSONGetMigrationStatus);
DO_TEST(qemuMonitorJSONGetSpiceMigrationStatus);
+DO_TEST(qemuMonitorJSONGetPtyPaths);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/18] qemumonitorjsontest: Test qemuMonitorJSONGetSpiceMigrationStatus
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 40
1 file changed, 40 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 576288b..b6adb35 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1688,6 +1688,45 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetSpiceMigrationStatus(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+bool spiceMigrated;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-spice,
+ {
+ \return\: {
+ \migrated\: true,
+ \enabled\: false,
+ \mouse-mode\: \client\
+ },
+ \id\: \libvirt-14\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorTestGetMonitor(test),
+ spiceMigrated) 0)
+goto cleanup;
+
+if (!spiceMigrated) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Invalid spice migration status: %d, expecting 1,
+ spiceMigrated);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1777,6 +1816,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetBlockStatsInfo);
DO_TEST(qemuMonitorJSONGetMigrationCacheSize);
DO_TEST(qemuMonitorJSONGetMigrationStatus);
+DO_TEST(qemuMonitorJSONGetSpiceMigrationStatus);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/18] qemumonitorjsontest: Test qemuMonitorJSONGetBalloonInfo
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 587b66f..ba2de45 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1214,6 +1214,42 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetBalloonInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+unsigned long long currmem;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-balloon,
+ {
+ \return\: {
+ \actual\: 4294967296
+ },
+ \id\: \libvirt-9\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetBalloonInfo(qemuMonitorTestGetMonitor(test),
currmem) 0)
+goto cleanup;
+
+if (currmem != (4294967296/1024)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Unexpected currmem value: %llu, currmem);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1298,6 +1334,7 @@ mymain(void)
DO_TEST_GEN(qemuMonitorJSONDetachCharDev);
DO_TEST(qemuMonitorJSONGetCPUInfo);
DO_TEST(qemuMonitorJSONGetVirtType);
+DO_TEST(qemuMonitorJSONGetBalloonInfo);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/18] qemumonitorjsontest: Test qemuMonitorJSONGetTargetArch
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 9e9dbcc..5636fb6 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1862,6 +1862,43 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetTargetArch(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+char *arch;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-target,
+ {
+ \return\: {
+ \arch\: \x86_64\
+ },
+ \id\: \libvirt-21\
+ }) 0)
+goto cleanup;
+
+if (!(arch =
qemuMonitorJSONGetTargetArch(qemuMonitorTestGetMonitor(test
+goto cleanup;
+
+if (STRNEQ(arch, x86_64)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Unexpected architecture %s, expecting x86_64,
+ arch);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(arch);
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1955,6 +1992,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetPtyPaths);
DO_TEST(qemuMonitorJSONSendKey);
DO_TEST(qemuMonitorJSONSetBlockIoThrottle);
+DO_TEST(qemuMonitorJSONGetTargetArch);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 15/18] qemumonitorjsontest: Test qemuMonitorJSONSendKey
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 14c64fd..2c5c452 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1793,6 +1793,31 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONSendKey(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+unsigned int keycodes[] = {43, 26, 46, 46, 32};
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, send-key,
+ {\return\: {}, \id\: \libvirt-16\}) 0)
+goto cleanup;
+
+if (qemuMonitorJSONSendKey(qemuMonitorTestGetMonitor(test),
+ 0, keycodes, ARRAY_CARDINALITY(keycodes)) 0)
+goto cleanup;
+
+ret = 0;
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1884,6 +1909,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetMigrationStatus);
DO_TEST(qemuMonitorJSONGetSpiceMigrationStatus);
DO_TEST(qemuMonitorJSONGetPtyPaths);
+DO_TEST(qemuMonitorJSONSendKey);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/18] qemumonitorjsontest: Introduce some tests
and fix some bugs I've ran into while writing the tests. Michal Privoznik (18): qemuMonitorJSONGetVirtType: Fix error message qemumonitorjsontest: Test qemuMonitorJSONSystemPowerdown qemuMonitorJSONSendKey: Avoid double free qemuMonitorTestFree: Join worker thread unconditionally qemumonitorjsontest: Extend the test for yet another monitor commands qemumonitorjsontest: Test qemuMonitorJSONGetCPUInfo qemumonitorjsontest: Test qemuMonitorJSONGetVirtType qemumonitorjsontest: Test qemuMonitorJSONGetBalloonInfo qemumonitorjsontest: Test qemuMonitorJSONGetBlockInfo qemumonitorjsontest: Test qemuMonitorJSONGetBlockStatsInfo qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCacheSize qemumonitorjsontest: Test qemuMonitorJSONGetMigrationStatus qemumonitorjsontest: Test qemuMonitorJSONGetSpiceMigrationStatus qemumonitorjsontest: Test qemuMonitorJSONGetPtyPaths qemumonitorjsontest: Test qemuMonitorJSONSendKey qemumonitorjsontest: Test qemuMonitorJSONSetBlockIoThrottle qemumonitorjsontest: Test qemuMonitorJSONGetTargetArch qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCapability src/qemu/qemu_monitor_json.c | 5 +- tests/qemumonitorjsontest.c | 955 ++- tests/qemumonitortestutils.c | 3 +- 3 files changed, 959 insertions(+), 4 deletions(-) -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/18] qemumonitorjsontest: Test qemuMonitorJSONSystemPowerdown
Right now, we are testing qemuMonitorSystemPowerdown instead of qemuMonitorJSONSystemPowerdown. It makes no harm, as both functions have the same header and the former is just a wrapper over the latter. But we should be consistent as we're testing the JSON functions only in here. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0fb8d65..6f218ea 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1080,7 +1080,7 @@ mymain(void) DO_TEST(GetDeviceAliases); DO_TEST(CPU); DO_TEST_SIMPLE(qmp_capabilities, qemuMonitorJSONSetCapabilities); -DO_TEST_SIMPLE(system_powerdown, qemuMonitorSystemPowerdown); +DO_TEST_SIMPLE(system_powerdown, qemuMonitorJSONSystemPowerdown); DO_TEST_SIMPLE(system_reset, qemuMonitorJSONSystemReset); DO_TEST_SIMPLE(migrate_cancel, qemuMonitorJSONMigrateCancel); DO_TEST_SIMPLE(inject-nmi, qemuMonitorJSONInjectNMI); -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCacheSize
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 7ba8ad2..52df486 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1603,6 +1603,42 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetMigrationCacheSize(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+unsigned long long cacheSize;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-migrate-cache-size,
+ {
+ \return\: 67108864,
+ \id\: \libvirt-12\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetMigrationCacheSize(qemuMonitorTestGetMonitor(test),
+ cacheSize) 0)
+goto cleanup;
+
+if (cacheSize != 67108864) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Invalid cacheSize: %llu, expected 67108864,
+ cacheSize);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+qemuMonitorTestFree(test);
+return ret;
+}
+
+static int
mymain(void)
{
int ret = 0;
@@ -1690,6 +1726,7 @@ mymain(void)
DO_TEST(qemuMonitorJSONGetBalloonInfo);
DO_TEST(qemuMonitorJSONGetBlockInfo);
DO_TEST(qemuMonitorJSONGetBlockStatsInfo);
+DO_TEST(qemuMonitorJSONGetMigrationCacheSize);
virObjectUnref(xmlopt);
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/18] qemumonitorjsontest: Test qemuMonitorJSONGetBlockStatsInfo
While the reply can be reused test qemuMonitorJSONGetBlockExtent and
qemuMonitorJSONGetBlockExtent too.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 212
1 file changed, 212 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 7a851bd..7ba8ad2 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1392,6 +1392,217 @@ cleanup:
}
static int
+testQemuMonitorJSONqemuMonitorJSONGetBlockStatsInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+long long rd_req, rd_bytes, rd_total_times;
+long long wr_req, wr_bytes, wr_total_times;
+long long flush_req, flush_total_times, errs;
+int nparams;
+unsigned long long extent;
+
+const char *reply =
+{
+\return\: [
+{
+\device\: \drive-virtio-disk0\,
+\parent\: {
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 5256018944,
+\wr_total_time_ns\: 0,
+\wr_bytes\: 0,
+\rd_total_time_ns\: 0,
+\flush_operations\: 0,
+\wr_operations\: 0,
+\rd_bytes\: 0,
+\rd_operations\: 0
+}
+},
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 10406001664,
+\wr_total_time_ns\: 530699221,
+\wr_bytes\: 2845696,
+\rd_total_time_ns\: 640616474,
+\flush_operations\: 0,
+\wr_operations\: 174,
+\rd_bytes\: 28505088,
+\rd_operations\: 1279
+}
+},
+{
+\device\: \drive-virtio-disk1\,
+\parent\: {
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 0,
+\wr_total_time_ns\: 0,
+\wr_bytes\: 0,
+\rd_total_time_ns\: 0,
+\flush_operations\: 0,
+\wr_operations\: 0,
+\rd_bytes\: 0,
+\rd_operations\: 0
+}
+},
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 0,
+\wr_total_time_ns\: 0,
+\wr_bytes\: 0,
+\rd_total_time_ns\: 8232156,
+\flush_operations\: 0,
+\wr_operations\: 0,
+\rd_bytes\: 348160,
+\rd_operations\: 85
+}
+},
+{
+\device\: \drive-ide0-1-0\,
+\parent\: {
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 0,
+\wr_total_time_ns\: 0,
+\wr_bytes\: 0,
+\rd_total_time_ns\: 0,
+\flush_operations\: 0,
+\wr_operations\: 0,
+\rd_bytes\: 0,
+\rd_operations\: 0
+}
+},
+\stats\: {
+\flush_total_time_ns\: 0,
+\wr_highest_offset\: 0,
+\wr_total_time_ns\: 0,
+\wr_bytes\: 0,
+\rd_total_time_ns\: 1004952,
+\flush_operations\: 0,
+\wr_operations\: 0,
+\rd_bytes\: 49250,
+\rd_operations\: 16
+}
+}
+],
+\id\: \libvirt-11\
+};
+
+if (!test)
+return -1;
+
+/* fill in seven times - we are gonna ask seven times later on */
+if (qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
+qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
+qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
+qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
+qemuMonitorTestAddItem(test, query-blockstats, reply) 0 ||
+qemuMonitorTestAddItem(test,
Re: [libvirt] [PATCH 04/18] qemuMonitorTestFree: Join worker thread unconditionally
On Wed, Oct 02, 2013 at 07:09:53PM +0200, Michal Privoznik wrote: The thread needs to be joined no matter if it was still running when qemuMonitorTestFree is called or not. The worker is thread spawned in qemuMonitorTestNew() and has to be joined. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitortestutils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index cd43c7b..4989183 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -354,8 +354,7 @@ qemuMonitorTestFree(qemuMonitorTestPtr test) virObjectUnref(test-vm); -if (test-running) -virThreadJoin(test-thread); +virThreadJoin(test-thread); if (timer != -1) virEventRemoveTimeout(timer); qemuMonitorTestNew() will call virMonitorTestFree() from an 'error' label and this label can be reached even from places where virThreadCreate was not yet run. So I think this will cause a hang in those cases. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/18] qemuMonitorJSONGetVirtType: Fix error message
On 10/02/2013 11:09 AM, Michal Privoznik wrote:
When querying for kvm, we try to find 'enabled' field. Hence the error
message should report we haven't found 'enabled' and not 'running'
(which is not even in the reply). Probably a typo or copy-paste error.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/qemu/qemu_monitor_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 2d84161..fc57c00 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1300,7 +1300,7 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
if (virJSONValueObjectGetBoolean(data, enabled, val) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(info kvm reply missing 'running' field));
+ _(info kvm reply missing 'enabled' field));
ret = -1;
goto cleanup;
}
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/18] qemumonitorjsontest: Test qemuMonitorJSONSystemPowerdown
On 10/02/2013 11:09 AM, Michal Privoznik wrote: Right now, we are testing qemuMonitorSystemPowerdown instead of qemuMonitorJSONSystemPowerdown. It makes no harm, as both functions have the same header and the former is just a wrapper over the latter. But we should be consistent as we're testing the JSON functions only in here. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Actually, I think we should do the opposite, and test only the wrapper functions that the rest of qemu_driver.c and friends will be calling. For example, testQemuMonitorJSONGetVersion() calls into qemuMonitorGetVersion, not qemuMonitorJSONGetVersion. Besides, I can't help but wonder if we should someday switch qemu_monitor.c into a callback struct, rather than it's current hard-coded callouts into _text or _json, and where qemu_monitor_json.h could be drastically reduced in size (just a few functions needed to register the callback driver, and most of the functions listed as static and just plug their address into the callback struct) - it would make for less duplication work when adding support for a new QMP command. But for that to work, the qemuMonitorJSON functions can't be called by the testsuite. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/18] qemuMonitorJSONSendKey: Avoid double free
On 10/02/2013 11:09 AM, Michal Privoznik wrote: After successful @cmd construction the memory where @keys points to is part of @cmd. Avoid double freeing it. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_monitor_json.c | 3 +++ 1 file changed, 3 insertions(+) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/18] qemumonitorjsontest: Extend the test for yet another monitor commands
On 10/02/2013 11:09 AM, Michal Privoznik wrote:
So far, we're unit testing some basic functions and some (so called)
simple functions (e.g. qmp_capabilities, system_powerdown). However,
there are more functions which expect simple {'return': {}} reply, but
takes more args to construct the command (for instance set_link). This
patch aims on such functions.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 97
+
1 file changed, 97 insertions(+)
+#define GEN_TEST_FUNC(funcName, ...)\
+static int \
+testQemuMonitorJSON ## funcName(const void *opaque) \
+{ \
+const testQemuMonitorJSONSimpleFuncDataPtr data = \
+(const testQemuMonitorJSONSimpleFuncDataPtr) opaque;\
+virDomainXMLOptionPtr xmlopt = data-xmlopt;\
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); \
+const char *reply = data-reply;\
+int ret = -1; \
+\
+if (!test) \
+return -1; \
+\
+if (!reply) \
+reply = {\return\:{}}; \
+\
+if (qemuMonitorTestAddItem(test, data-cmd, reply) 0) \
+goto cleanup; \
+\
+if (funcName(qemuMonitorTestGetMonitor(test), __VA_ARGS__) 0) \
Again, I think we should be calling the public function in
qemu_monitor.h, not the version in qemu_monitor_json. To do that,
you'll need to do something like:
#define GEN_TEST_FUNC(funcSuffix, ...)
...
testQemuMonitor ## funcSuffix()
...
if (qemuMonitor ## funcSuffix(qemuMonitorTestGetMonitor(test),
__VA_ARGS__) 0
+
+GEN_TEST_FUNC(qemuMonitorJSONSetLink, vnet0,
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN)
and call it like:
GEN_TEST_FUNC(SetLink, vnet0, VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN)
+GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, vdb, /foo/bar, NULL, 1024,
+ VIR_DOMAIN_BLOCK_REBASE_SHALLOW
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
Umm - that's a fancy way to write 0. Did you mean to s//|/
@@ -1086,6 +1154,35 @@ mymain(void)
DO_TEST_SIMPLE(inject-nmi, qemuMonitorJSONInjectNMI);
DO_TEST_SIMPLE(system_wakeup, qemuMonitorJSONSystemWakeup);
DO_TEST_SIMPLE(nbd-server-stop, qemuMonitorJSONNBDServerStop);
+DO_TEST_GEN(qemuMonitorJSONSetLink);
Again, I think you want:
DO_TEST_GEN(SetLink);
to go through the public wrapper.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/18] qemumonitorjsontest: Test qemuMonitorJSONSystemPowerdown
On Wed, Oct 02, 2013 at 11:42:13AM -0600, Eric Blake wrote: On 10/02/2013 11:09 AM, Michal Privoznik wrote: Right now, we are testing qemuMonitorSystemPowerdown instead of qemuMonitorJSONSystemPowerdown. It makes no harm, as both functions have the same header and the former is just a wrapper over the latter. But we should be consistent as we're testing the JSON functions only in here. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Actually, I think we should do the opposite, and test only the wrapper functions that the rest of qemu_driver.c and friends will be calling. For example, testQemuMonitorJSONGetVersion() calls into qemuMonitorGetVersion, not qemuMonitorJSONGetVersion. Well this test suite was specifically targetting only the JSON monitor impl, not the text mode impl, so calling the JSON functions is right IMHO. There is separate testing for the text mode monitor Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ANNOUNCE: libvirt 1.0.5.6 maintenance release
On Wed, Oct 02, 2013 at 10:59:03AM +0200, Christophe Fergeau wrote: On Mon, Sep 23, 2013 at 01:36:40PM -0400, Cole Robinson wrote: I basically do: - quick smoke test to make sure libvirt and virsh are working: -- stop system libvirtd -- sudo ./daemon/libvirt -- sudo ./tools/virsh list --all - make check make rpm make distcheck - cd po/ make update-po - bump configure.ac - bump rpm spec - git tag v$VER -m libvirt release $VER - commit - git push git push --tags - git clean -xdf ./autogen.sh --system make dist - upload tarball - update http://wiki.libvirt.org/page/Maintenance_Releases - send out announce mail It would be very nice if the announce mail included either a sha256 checksum for the tarball (thanks Guido for doing that in your recent release!), or a GPG key used to sign the release, see http://lwn.net/Articles/548857/ for more details about this. I sign the tag as well as the email so together with the checksums this builds the chain of trust (given you trust my gpg signature). A further improvement would be to build the tarballs first and then add the checksums as the commit message of the tag - I'll try to do that with the next 0.9.12.x release. This is the quick hack I'm currently using based on Cole's description - it sure needs further tweeking when doing further releases: --- #!/bin/bash # set -e if [ -z $1 -o -z $2 ]; then echo Usage $0 version oldversion exit 1 fi VERSION=$1 OLDVER=$2 TARBALL=libvirt-$1.tar.gz MD5=$(md5sum $TARBALL) SHA1=$(sha1sum $TARBALL) SHA256=$(sha256sum $TARBALL) git tag -s -m libvirt release $VERSION v$VERSION CHANGES=$(git log --no-merges --pretty=%s (%an) v$OLDVER..v$VERSION) cat EOF To: libvirt-l...@redhat.com Cc: libvirt-annou...@redhat.com Subject: ANNOUNCE: libvirt $VERSION maintenance release libvirt $VERSION maintenance release is now available. This is libvirt 0.9.12 with additional bugfixes that have accumulated upstream since the initial release. This release can be downloaded at: http://libvirt.org/sources/stable_updates/$TARBALL md5sum: $MD5 sha1: $SHA1 sha256: $SHA256 Changes in this release: $CHANGES For info about past maintenance releases, see: http://wiki.libvirt.org/page/Maintenance_Releases Cheers, -- Guido EOF -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/18] qemumonitorjsontest: Test qemuMonitorJSONGetCPUInfo
On 10/02/2013 11:09 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 77
+
1 file changed, 77 insertions(+)
+ {
+ \current\: false,
+ \CPU\: 2,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17624
+ },
+ {
+ \current\: false,
+ \CPU\: 3,
+ \pc\: -2130530478,
+ \halted\: true,
+ \thread_id\: 17625
+ }
In the past, we've had a bug report about the case when cpu ids returned
by this command are not contiguous. Should you try that here, to make
sure we've handled it correctly?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/18] qemumonitorjsontest: Test qemuMonitorJSONGetVirtType
On 10/02/2013 11:09 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 38 ++
1 file changed, 38 insertions(+)
static int
+testQemuMonitorJSONqemuMonitorJSONGetVirtType(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+int virtType;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddItem(test, query-kvm,
+ {
+ \return\: {
+ \enabled\: true,
+ \present\: true
+ },
+ \id\: \libvirt-8\
+ }) 0)
+goto cleanup;
+
+if (qemuMonitorJSONGetVirtType(qemuMonitorTestGetMonitor(test),
virtType) 0)
+goto cleanup;
+
+if (virtType != VIR_DOMAIN_VIRT_KVM) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Unexpected virt type: %d, virtType);
+goto cleanup;
+}
This should also test the case where the command returns failure
(commandnotfound) and/or returns success but enabled:false, to make
sure we don't report VIR_DOMAIN_VIRT_KVM in those cases.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/18] qemumonitorjsontest: Test qemuMonitorJSONGetBalloonInfo
On 10/02/2013 11:09 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 37 + 1 file changed, 37 insertions(+) + +if (qemuMonitorJSONGetBalloonInfo(qemuMonitorTestGetMonitor(test), currmem) 0) +goto cleanup; I guess Dan is in favor of the direct calls, in which case this is fine (I'd still rather go through the wrappers, so we can get rid of duplication in qemu_monitor_json.h). The rest of the test looks fine, so I'm not going to hold up an improvement to the testsuite in favor of a refactoring not yet done. ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/18] qemumonitorjsontest: Test qemuMonitorJSONGetBlockInfo
On 10/02/2013 11:09 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 143 1 file changed, 143 insertions(+) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/18] qemumonitorjsontest: Test qemuMonitorJSONGetBlockStatsInfo
On 10/02/2013 11:09 AM, Michal Privoznik wrote: While the reply can be reused test qemuMonitorJSONGetBlockExtent and qemuMonitorJSONGetBlockExtent too. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 212 1 file changed, 212 insertions(+) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCacheSize
On 10/02/2013 11:10 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 37 + 1 file changed, 37 insertions(+) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationStatus
On 10/02/2013 11:10 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 50 + 1 file changed, 50 insertions(+) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/18] qemumonitorjsontest: Test qemuMonitorJSONGetPtyPaths
On 10/02/2013 11:10 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 67 + 1 file changed, 67 insertions(+) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/18] qemumonitorjsontest: Test qemuMonitorJSONGetSpiceMigrationStatus
On 10/02/2013 11:10 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 40 1 file changed, 40 insertions(+) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/18] qemumonitorjsontest: Test qemuMonitorJSONSendKey
On 10/02/2013 11:10 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/qemumonitorjsontest.c | 26 ++
1 file changed, 26 insertions(+)
static int
+testQemuMonitorJSONqemuMonitorJSONSendKey(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
+int ret = -1;
+unsigned int keycodes[] = {43, 26, 46, 46, 32};
Is sending the same keycode more than once in a single batch really
supported? The guest will see the key pushed only once.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 18/18] qemumonitorjsontest: Test qemuMonitorJSONGetMigrationCapability
On 10/02/2013 11:10 AM, Michal Privoznik wrote: And so do qemuMonitorJSONSetMigrationCapability. s/And so/Also/ Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 46 + 1 file changed, 46 insertions(+) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] Don't set netdev offline in container cleanup
On 10/02/2013 05:31 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
During container cleanup there is a race where the kernel may
have destroyed the veth device before we try to set it offline.
This causes log error messages. Given that we're about to
delete the device entirely, setting it offline is pointless.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 1 -
1 file changed, 1 deletion(-)
ACK.
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index f92c613..a78784e 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -193,7 +193,6 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
virDomainNetDefPtr iface = vm-def-nets[i];
vport = virDomainNetGetActualVirtPortProfile(iface);
if (iface-ifname) {
-ignore_value(virNetDevSetOnline(iface-ifname, false));
if (vport
vport-virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)
ignore_value(virNetDevOpenvswitchRemovePort(
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Avoid reporting an error if veth device is already deleted
On 10/02/2013 05:31 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The kernel automatically destroys veth devices when cleaning up the container network namepace. During normal shutdown, it s/namepace/namespace/ is thus likely that the attempt to run 'ip link del vethN' will fail. If it fails, check if the device exists, and avoid reporting an error if it has gone. This switches to use the virCommand APIs instead of virRun too. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/util/virnetdevveth.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs
So we can get the MAC addr if we use the '--dhcp-script' parameter to
make dnsmasq invoke a helper program we create, to save the lease
details we need.
Incidentally, I see our XML format is actually wrong, because it says
the 'mac' attribute is forbidden for host elements using IPv6. dnsmasq
actually allows you to specify either mac addr or IAID identifiers for
IPv6 dhcp host entries.
I gave a try to the --dhcp-script option of dnsmasq. Following are the findings:
Script used (a little modified version of
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=blob_plain;f=contrib/mactable/macscript;hb=HEAD):
#!/bin/bash
STATUS_FILE=/var/lib/libvirt/dnsmasq/dnsmasq-ip-mac.status
# Script for dnsmasq lease-change hook.
# Maintains the above file with a IP address/MAC address pairs,
# one lease per line. Works with IPv4 and IPv6 leases, file is
# atomically updated, so no races for users of the data.
action=$1
mac=$2 # IPv4
ip=$3
expirytime=$DNSMASQ_LEASE_EXPIRES
hostname=$DNSMASQ_SUPPLIED_HOSTNAME
clientid=$DNSMASQ_CLIENT_ID
# ensure it always exists.
if [ ! -f $STATUS_FILE ]; then
touch $STATUS_FILE
fi
if [ -n $DNSMASQ_IAID ]; then
mac=$DNSMASQ_MAC # IPv6
clientid=$2
fi
# worry about an add or old action when the MAC address is not known:
# leave any old one in place in that case.
if [ $action = add -o $action = old -o $action = del ]; then
if [ -n $mac -o $action = del ]; then
sed /^${ip//./\.} / d $STATUS_FILE $STATUS_FILE.new
if [ $action = add -o $action = old ]; then
echo $expirytime $mac $ip $hostname $clientid $STATUS_FILE.new
fi
mv $STATUS_FILE.new $STATUS_FILE # atomic update.
fi
fi
Changes made to libvirt code:
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8787bdb..7f9a74f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1058,6 +1058,7 @@
networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
virCommandAddArgFormat(cmd, --conf-file=%s, configfile);
+virCommandAddArgFormat(cmd, --dhcp-script=%s,
/var/lib/libvirt/dnsmasq/macscript.sh);
*cmdout = cmd;
ret = 0;
cleanup:
In dnsmasq version 2.65 (latest on f18 repos), useful variables that were set:
In the case of ipv4:
$1=add $2=52:54:00:95:41:5d $3=192.168.100.128
DNSMASQ_INTERFACE=virbr0
DNSMASQ_TAGS=virbr0
DNSMASQ_TIME_REMAINING=3600
DNSMASQ_LEASE_EXPIRES=1380745674
In the case of ipv6:
$1=add $2=00:01:00:01:19:df:2e:19:52:54:00:24:13:15 $3=2001:db8:ca2:2:1::45
DNSMASQ_INTERFACE=virbr3
DNSMASQ_TAGS=dhcpv6 virbr3
DNSMASQ_SERVER_DUID=00:01:00:01:19:df:29:7e:f0:4d:a2:8c:14:51
DNSMASQ_IAID=2364181
DNSMASQ_TIME_REMAINING=3600
DNSMASQ_LEASE_EXPIRES=1380745131
In the latest dnsmasq version 2.67rc2-3-g889d8a1 (built after cloning
from git://thekelleys.org.uk/dnsmasq.git), useful variables that were
set:
In the case of ipv4:
add 52:54:00:1a:a1:55 192.168.100.204
DNSMASQ_INTERFACE=virbr0
DNSMASQ_LEASE_EXPIRES=1380749702
DNSMASQ_TAGS=virbr0
DNSMASQ_TIME_REMAINING=3600
In the case of ipv6:
add 00:01:00:01:19:df:3a:8e:52:54:00:7d:49:25 2001:db8:ca2:2:1::f5
DNSMASQ_IAID=8210725
DNSMASQ_INTERFACE=virbr3
DNSMASQ_LEASE_EXPIRES=1380748320
DNSMASQ_MAC=52:54:00:7d:49:25
DNSMASQ_SERVER_DUID=
DNSMASQ_TAGS=dhcpv6 virbr3
DNSMASQ_TIME_REMAINING=3600
So, in case of latest dnsmasq code, output in dnsmasq-ip-mac.status:
1380747917 52:54:00:82:5e:09 2001:db8:ca2:2:1::79
1380747943 52:54:00:61:bd:d8 2001:db8:ca2:2:1::88
1380748110 52:54:00:15:1e:05 192.168.100.180
1380748320 52:54:00:7d:49:25 2001:db8:ca2:2:1::f5
00:01:00:01:19:df:3a:8e:52:54:00:7d:49:25
1380749702 52:54:00:1a:a1:55 192.168.100.204
1380749877 52:54:00:73:0a:27 192.168.100.190
1380749879 52:54:00:b7:87:3e 2001:db8:ca2:2:1::3e
00:01:00:01:19:df:40:a6:52:54:00:b7:87:3e
1380749880 52:54:00:bc:55:df 2001:db8:ca2:2:1::8f
00:01:00:01:19:df:40:a6:52:54:00:bc:55:df
1380749880 52:54:00:b7:87:3e 2001:db8:ca2:2:1::3e
00:01:00:01:19:df:40:a6:52:54:00:b7:87:3e
So, I think it is OK to parse the custom generated file:
dnsmasq-ip-mac.status (after deciding its exact format). But the only
issue right now is that the variable DNSMASQ_MAC for DHCPv6 is set
only in the case of latest dnsmasq code (I don't think it is even
available in the tarballs yet)
--
Nehal J Wani
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] Avoid deleting NULL veth device name
On 10/02/2013 05:31 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
If veth device allocation has a fatal error, the veths
array may contain NULL device names. Avoid calling the
virNetDevVethDelete function on such names.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index a78784e..d07ff13 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1290,7 +1290,7 @@ cleanup:
rc = -1;
}
for (i = 0; i nveths; i++) {
-if (rc != 0)
+if (rc != 0 veths[i])
ignore_value(virNetDevVethDelete(veths[i]));
VIR_FREE(veths[i]);
}
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] Retry veth device creation on failure
On 10/02/2013 05:31 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The veth device creation code run in two steps, first it looks
for two free veth device names, then it runs ip link to create
the veth pair. There is an obvious race between finding free
names and creating them, when guests are started in parallel.
Rewrite the code to loop and re-try creation if it fails, to
deal with the race condition.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virnetdevveth.c | 151
+--
1 file changed, 92 insertions(+), 59 deletions(-)
/* Functions */
+
+static int virNetDevVethExists(int devNum)
+{
+int ret;
+char *path = NULL;
+if (virAsprintf(path, /sys/class/net/veth%d/, devNum) 0)
+return -1;
+ret = virFileExists(path) ? 1 : 0;
Looks a bit funny for converting a bool to int. I probably would have
skipped the ternary; but what you did is not wrong.
-if (virAsprintf(veth, veth%d, devNum) 0)
-return -1;
+for (devNum = startDev ; devNum MAX_DEV_NUM ; devNum++) {
How did this get past 'make syntax-check'? We cleaned up the code base
to avoid space before ';'.
ACK with the syntax cleaned up.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] Use 'vnet' as prefix for veth devices
On 10/02/2013 05:31 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The XML parser reserves 'vnet' as a prefix for automatically generated NIC device names. Switch the veth device creation to use this prefix, so it does not have to worry about clashes with user specified names in the XML. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/util/virnetdevveth.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/18] qemumonitorjsontest: Test qemuMonitorJSONGetTargetArch
On 10/02/2013 11:10 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 38 ++ 1 file changed, 38 insertions(+) ACK -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/18] qemumonitorjsontest: Test qemuMonitorJSONSetBlockIoThrottle
On 10/02/2013 11:10 AM, Michal Privoznik wrote: And so do qemuMonitorJSONGetBlockIoThrottle. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemumonitorjsontest.c | 185 +++- 1 file changed, 115 insertions(+), 70 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
