Re: [libvirt] [PATCH] fix deadlock when qemu cannot start
On Fri, Mar 16, 2012 at 02:37:42PM +0800, Wen Congyang wrote:
When qemu cannot start, we may call qemuProcessStop() twice.
We have check whether the vm is running at the beginning of
qemuProcessStop() to avoid libvirt deadlock. We call
qemuProcessStop() with driver and vm locked. It seems that
we can avoid libvirt deadlock. But unfortunately we may
unlock driver and vm in the function qemuProcessKill() while
vm-def-id is not -1. So qemuProcessStop() will be run twice,
and monitor will be freed unexpectedly. So we should set
vm-def-id to -1 at the beginning of qemuProcessStop().
---
src/qemu/qemu_process.c | 20 ++--
src/qemu/qemu_process.h |2 ++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0af3751..44814df 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver,
VIR_DEBUG(vm=%s pid=%d flags=%x,
vm-def-name, vm-pid, flags);
-if (!virDomainObjIsActive(vm)) {
-VIR_DEBUG(VM '%s' not active, vm-def-name);
-return 0;
-}
+if (!(flags VIR_QEMU_PROCESS_KILL_NOCHECK))
+if (!virDomainObjIsActive(vm)) {
+VIR_DEBUG(VM '%s' not active, vm-def-name);
+return 0;
+}
/* This loop sends SIGTERM (or SIGKILL if flags has
* VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
@@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver,
return;
}
+/*
+ * We may unlock driver and vm in qemuProcessKill(), so the other thread
+ * can lock driver and vm, and then call qemuProcessStop(). So we should
+ * set vm-def-id to -1 here to avoid qemuProcessStop() called twice.
+ */
+vm-def-id = -1;
+
if ((logfile = qemuDomainCreateLog(driver, vm, true)) 0) {
/* To not break the normal domain shutdown process, skip the
* timestamp log writing if failed on opening log file. */
@@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver,
}
/* shut it off for sure */
-ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE|
+ VIR_QEMU_PROCESS_KILL_NOCHECK));
/* Stop autodestroy in case guest is restarted */
qemuProcessAutoDestroyRemove(driver, vm);
@@ -3892,7 +3901,6 @@ retry:
vm-taint = 0;
vm-pid = -1;
-vm-def-id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
VIR_FREE(priv-vcpupids);
priv-nvcpupids = 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 761db6f..891f7a5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn,
typedef enum {
VIR_QEMU_PROCESS_KILL_FORCE = 1 0,
VIR_QEMU_PROCESS_KILL_NOWAIT = 1 1,
+ VIR_QEMU_PROCESS_KILL_NOCHECK = 1 2, /* donot check whether the vm is
+ running */
} virQemuProcessKillMode;
int qemuProcessKill(struct qemud_driver *driver,
Hi Wen,
sorry for the delay in reviewing the patch. I think I understand the
issue, and removing the pid and using an extra flag to qemuProcessKill
to avoid the race sounds reasonable. I rebased the patch a bit, made
some cosmetic changes especially on the comments and pushed it so it is
included in rc2 for more testing,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On 03/29/2012 05:52 PM, Daniel P. Berrange wrote:
On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote:
+if (oldbridge
+virNetDevBridgeRemovePort(oldbridge, olddev-ifname) 0) {
+return -1;
+}
+if (virNetDevBridgeAddPort(newbridge, olddev-ifname) 0) {
+if (virNetDevBridgeAddPort(oldbridge, olddev-ifname) 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to recover former state by adding
port
+ to bridge %s), oldbridge);
+}
+return -1;
+}
I think you need to emit 2 audit notifications here, one for the bridge
being removed and one for the bridge being added.
That does sound like a good idea, but the current virDomainAuditNet()
function only reports MAC address, and virDomainAuditNetDevice() only
reports /dev/net/tun - neither of them gives any information about the
name of tap device or which bridge it is being attached to.
Right now, here are the audit messages that are logged when I do a full
device detach/attach of a network device:
type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=detach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=52:54:00:00:01:81 new-net=?: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/net/tun rdev=0A:C8:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/vhost-net rdev=0A:EE:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=attach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=? new-net=52:54:00:00:01:81: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
It does a good job of telling me the MAC address that's going to be used
by the domain, but nothing about how it's connected to the network.
If we're staying within the current boundaries of reporting, is there
really value in logging a pair of messages that are ultimately just
telling us that the same mac address was detached then immediately
reattached, but not saying anything about what it was connected to?
Alternately, if we're going to start reporting about changes in network
connection, shouldn't we also be reporting what those connections are to
begin with? (I think that's a change in scope of what's being audited,
and requires a separate patch if we decide we want to do that).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: set default name for SPICE agent channel when generating command
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).
This patch reverts commit b0e2bb33 and replaces it with the alternate
implementation of simply inserting the default value in the
appropriate place on the qemu commandline when no value is provided.
---
(Playing the devil's advocate on my own patch for a moment - one
advantage of Christophe's method of setting the default is that if we
use that object somewhere else in libvirt's code in the future, the
value will be set even if the XML left it unset, but with my method we
will have to check for a NULL name and replace it with the default
value anywhere we want to use it. So I won't say that either method is
definitely the proper way to go, but will just present this
alternative and see if someone else has an even stronger opinion than
me :-)
(BTW, I think I've decided while typing this message that, if we
decide to change from the original method of setting default to this
new method, the change should be pushed as two separate patches - one
reverting the original, and another adding the new. It's too close to
morning for me to take the time to do that right now, though.)
src/conf/domain_conf.c |7 ---
src/qemu/qemu_command.c |6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..ea558bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5248,13 +5248,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
goto error;
} else {
def-source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT;
-if (!def-target.name) {
-def-target.name = strdup(com.redhat.spice.0);
-if (!def-target.name) {
-virReportOOMError();
-goto error;
-}
-}
}
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..50b1e6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3457,9 +3457,9 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) {
virBufferAsprintf(buf, ,chardev=char%s,id=%s,
dev-info.alias, dev-info.alias);
-if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-dev-target.name) {
-virBufferAsprintf(buf, ,name=%s, dev-target.name);
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) {
+virBufferAsprintf(buf, ,name=%s, dev-target.name
+ ? dev-target.name : com.redhat.spice.0);
}
} else {
virBufferAsprintf(buf, ,id=%s, dev-info.alias);
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: set default name for SPICE agent channel when generating command
Hey,
Patch looks good to me, thanks for writing it :)
Christophe
On Fri, Mar 30, 2012 at 03:50:37AM -0400, Laine Stump wrote:
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).
This patch reverts commit b0e2bb33 and replaces it with the alternate
implementation of simply inserting the default value in the
appropriate place on the qemu commandline when no value is provided.
---
(Playing the devil's advocate on my own patch for a moment - one
advantage of Christophe's method of setting the default is that if we
use that object somewhere else in libvirt's code in the future, the
value will be set even if the XML left it unset, but with my method we
will have to check for a NULL name and replace it with the default
value anywhere we want to use it. So I won't say that either method is
definitely the proper way to go, but will just present this
alternative and see if someone else has an even stronger opinion than
me :-)
(BTW, I think I've decided while typing this message that, if we
decide to change from the original method of setting default to this
new method, the change should be pushed as two separate patches - one
reverting the original, and another adding the new. It's too close to
morning for me to take the time to do that right now, though.)
src/conf/domain_conf.c |7 ---
src/qemu/qemu_command.c |6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..ea558bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5248,13 +5248,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
goto error;
} else {
def-source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT;
-if (!def-target.name) {
-def-target.name = strdup(com.redhat.spice.0);
-if (!def-target.name) {
-virReportOOMError();
-goto error;
-}
-}
}
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..50b1e6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3457,9 +3457,9 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) {
virBufferAsprintf(buf, ,chardev=char%s,id=%s,
dev-info.alias, dev-info.alias);
-if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-dev-target.name) {
-virBufferAsprintf(buf, ,name=%s, dev-target.name);
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) {
+virBufferAsprintf(buf, ,name=%s, dev-target.name
+ ? dev-target.name : com.redhat.spice.0);
}
} else {
virBufferAsprintf(buf, ,id=%s, dev-info.alias);
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
pgpimzUk9IT7d.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage backend: Add RBD (RADOS Block Device) support
This patch adds support for a new storage backend with RBD support.
RBD is the RADOS Block Device and is part of the Ceph distributed storage
system.
It comes in two flavours: Qemu-RBD and Kernel RBD, this storage backend only
supports
Qemu-RBD, thus limiting the use of this storage driver to Qemu only.
To function this backend relies on librbd and librados being present on the
local system.
The backend also supports Cephx authentication for safe authentication with the
Ceph cluster.
For storing credentials it uses the build-in secret mechanism of libvirt.
Signed-off-by: Wido den Hollander w...@widodh.nl
---
configure.ac | 20 ++
include/libvirt/libvirt.h.in |1 +
src/Makefile.am |9 +
src/conf/storage_conf.c | 197 ++---
src/conf/storage_conf.h | 16 +
src/storage/storage_backend.c|6 +
src/storage/storage_backend_rbd.c| 465 ++
src/storage/storage_backend_rbd.h| 30 ++
tests/storagepoolxml2xmlin/pool-rbd.xml | 11 +
tests/storagepoolxml2xmlout/pool-rbd.xml | 15 +
tools/virsh.c|7 +
11 files changed, 734 insertions(+), 43 deletions(-)
create mode 100644 src/storage/storage_backend_rbd.c
create mode 100644 src/storage/storage_backend_rbd.h
create mode 100644 tests/storagepoolxml2xmlin/pool-rbd.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-rbd.xml
diff --git a/configure.ac b/configure.ac
index 32cc8d0..b693e5b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1743,6 +1743,8 @@ AC_ARG_WITH([storage-mpath],
AC_HELP_STRING([--with-storage-mpath], [with mpath backend for the storage
driver @:@default=check@:@]),[],[with_storage_mpath=check])
AC_ARG_WITH([storage-disk],
AC_HELP_STRING([--with-storage-disk], [with GPartd Disk backend for the
storage driver @:@default=check@:@]),[],[with_storage_disk=check])
+AC_ARG_WITH([storage-rbd],
+ AC_HELP_STRING([--with-storage-rbd], [with RADOS Block Device backend for
the storage driver @:@default=check@:@]),[],[with_storage_rbd=check])
if test $with_libvirtd = no; then
with_storage_dir=no
@@ -1752,6 +1754,7 @@ if test $with_libvirtd = no; then
with_storage_scsi=no
with_storage_mpath=no
with_storage_disk=no
+ with_storage_rbd=no
fi
if test $with_storage_dir = yes ; then
AC_DEFINE_UNQUOTED([WITH_STORAGE_DIR], 1, [whether directory backend for
storage driver is enabled])
@@ -1910,6 +1913,22 @@ if test $with_storage_mpath = check; then
fi
AM_CONDITIONAL([WITH_STORAGE_MPATH], [test $with_storage_mpath = yes])
+if test $with_storage_rbd = yes || test $with_storage_rbd = check; then
+AC_CHECK_HEADER([rbd/librbd.h], [LIBRBD_FOUND=yes; break;])
+
+LIBRBD_LIBS=-lrbd -lrados -lcrypto
+
+if test $LIBRBD_FOUND = yes; then
+with_storage_rbd=yes
+LIBS=$LIBS $LIBRBD_LIBS
+else
+with_storage_rbd=no
+fi
+
+AC_DEFINE_UNQUOTED([WITH_STORAGE_RBD], 1, [wether RBD backend for storage
driver is enabled])
+fi
+AM_CONDITIONAL([WITH_STORAGE_RBD], [test $with_storage_rbd = yes])
+
LIBPARTED_CFLAGS=
LIBPARTED_LIBS=
if test $with_storage_disk = yes ||
@@ -2673,6 +2692,7 @@ AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi])
AC_MSG_NOTICE([SCSI: $with_storage_scsi])
AC_MSG_NOTICE([ mpath: $with_storage_mpath])
AC_MSG_NOTICE([Disk: $with_storage_disk])
+AC_MSG_NOTICE([ RBD: $with_storage_rbd])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Security Drivers])
AC_MSG_NOTICE([])
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 499dcd4..ee1d5ec 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2324,6 +2324,7 @@ typedef enum {
VIR_STORAGE_VOL_FILE = 0, /* Regular file based volumes */
VIR_STORAGE_VOL_BLOCK = 1,/* Block based volumes */
VIR_STORAGE_VOL_DIR = 2, /* Directory-passthrough based volume */
+ VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS Block
Device) */
#ifdef VIR_ENUM_SENTINELS
VIR_STORAGE_VOL_LAST
diff --git a/src/Makefile.am b/src/Makefile.am
index a2aae9d..e4457c3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -498,6 +498,9 @@ STORAGE_DRIVER_MPATH_SOURCES =
\
STORAGE_DRIVER_DISK_SOURCES = \
storage/storage_backend_disk.h storage/storage_backend_disk.c
+STORAGE_DRIVER_RBD_SOURCES = \
+ storage/storage_backend_rbd.h storage/storage_backend_rbd.c
+
STORAGE_HELPER_DISK_SOURCES = \
storage/parthelper.c
@@ -1040,6 +1043,11 @@ if WITH_STORAGE_DISK
libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES)
endif
+if WITH_STORAGE_RBD
+libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_RBD_SOURCES)
+libvirt_la_LIBADD += $(LIBRBD_LIBS)
+endif
+
if
Re: [libvirt] [PATCH] build: silence recent syntax check violations
On 30.03.2012 05:23, Eric Blake wrote:
An upstream gnulib bug meant that some of our syntax checks
weren't being run. Fix up our offenders before we upgrade to
a newer gnulib.
* src/util/virnetdevtap.c (virNetDevTapCreate): Use flags.
* tests/lxcxml2xmltest.c (mymain): Strip useless ().
---
The gnulib bug was here:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
I tested this by temporarily using the fixed gnulib maint.mk.
Pushing under the trivial rule. I can't call it a build breaker,
because it won't break the build without a gnulib update.
I'm reluctant to update the .gnulib submodule this late in the
game without some review, as we've had bad luck with a submodule
update after the rc1 build in previous releases, so I'm saving
that for another day. Besides, I'm waiting for a review of a
patch that fixes ssize_t for mingw, and it isn't worth a gnulib
update without that fix.
src/util/virnetdevtap.c |7 +--
tests/lxcxml2xmltest.c |4 ++--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 0b3ac46..717b6ac 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -129,12 +129,14 @@ virNetDevProbeVnetHdr(int tapfd)
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int fd;
struct ifreq ifr;
int ret = -1;
+virCheckFlags(VIR_NETDEV_TAP_CREATE_VNET_HDR, -1);
+
This is incomplete; networkStartNetworkVirtual() pass
VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE here. If a gnulib check is
causing fail, we should disable that check. Testing for flags at public
API is something that *have to* be done; omitting test at this low layer
and setting ATTRIBUTE_UNUSED is something that is intentional and
therefore is harmless.
Therefore we should either exclude src/util/* from checking, or drop
u_int flags completely here as they are unused after all. Okay, we can
also make virCheckFlags complete.
if ((fd = open(/dev/net/tun, O_RDWR)) 0) {
virReportSystemError(errno, %s,
_(Unable to open /dev/net/tun, is tun module
loaded?));
@@ -237,8 +239,9 @@ cleanup:
#else /* ! TUNSETIFF */
int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
int *tapfd ATTRIBUTE_UNUSED,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
+virCheckFlags(0, -1);
virReportSystemError(ENOSYS, %s,
_(Unable to create TAP devices on this platform));
However, this is even worse. Instead of throwing ENOSYS - the only
correct error here - we can throw some spurious error about unsupported
flags. This makes me lean towards excluding src/util/* from the check
causing trouble.
return -1;
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index 558bd01..6a87939 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -99,7 +99,7 @@ mymain(void)
int ret = 0;
if ((caps = testLXCCapsInit()) == NULL)
-return (EXIT_FAILURE);
+return EXIT_FAILURE;
# define DO_TEST_FULL(name, is_different, inactive) \
do {\
@@ -124,7 +124,7 @@ mymain(void)
virCapabilitiesFree(caps);
-return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
+return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
VIRT_TEST_MAIN(mymain)
ACK to these two changes.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API] RFC: Stabilization of libvirt-test-API
On 03/29/2012 03:42 PM, Osier Yang wrote:
On 2012年03月29日 20:14, Martin Kletzander wrote:
Hi everyone,
following our minutes, I'd like to start a discussion on what should be
done with libvirt-test-API so we can say it's stable and usable.
I would like to stress out that everything mentioned here is just an
opinion and I don't mean to talk down to someone as it may have seemed
earlier.
I think we should get some ideas from everyone, mostly QE as they will
be (are) the ones using this the most (if I understood correctly), and
then I'll be happy to help getting the code to the agreed status. I was
thinking about this from the wrong way probably and changing the angle
from what I look at it (and knowing there is some deadline) makes me
think of few levels of changes, which when introduced, could speed up
the test development and code understandability.
So here are the things I would like to do definitely (the optional
things follow later on):
- fix hard-coded options into real options (e.g. commit 65449e)
Absolutely we should either change/destroy it or generalize it
as global config.
- fix some env_* and util* code (functions duplicated with different
behavior)
This should be caused by mutilple persons worked on that, but lacked
reviewing.
- fix or remove harmful and pointless code (at this point, when
creating domain on remote machine, be prepared for the test to fail with
any other user then root and with root, have backup of both local and
remote '/root/.ssh' directories as the contents will be erased!)
So this means test-API only supports qemu:///system testing now,
needs to be improved for qemu:///session too.
Also I'd guess there are cases which only considers the QEMU/KVM
driver testing. If so, we need to either generalize it or seperate
it (in case of it are too specific to generalize), perhaps seperate
directories for different drivers. But this should be the future
plan, what we should do currently is try to generalize the tests
as much as we could.
- fix method names for the {connect,domain,etc.}API (get_host_name vs.
lookupByUUID etc.)
Yes, we need the consistent function/variable name, also consistent
coding (including the comments on top of scripts) style.
The optional things:
- get rid of classes in lib and make just few utility functions
covering *only* the methods that do something else than call the same
method in underlying class from the libvirt module.
Agreed, it looks to me many of the lib functions just simple pass
parameter to the underlying libvirt-python API, that's just meaningless/
useless.
- get rid of the new exception (I don't see any other difference than
in the name, which can make a difference in except: clause, but it's
converted everywhere)
Agreed. Just like the classes method in lib, it's feet of snake, ;)
- be able to share variables between tests (connection object and
anything else)
Not sure what's your exact meaning, could you explain more?
- introduce new config file for tests (.ini format, can be parsed by
ConfigParser, same as env.cfg, define variables used throughout the test
specifications
Do you mean destroy current config parsing codes? if so, we need
to rewrite (or much modification) codes of generator too. Any
critical disadvantage you see in the current parsing/generator
codes? I'd think the pricinple of current parsing (no generator)
is right, though it might have bugs or disadvantages, we can
improve/extend it.
To answer two of your questions at once, I'll show you an example of
what I had in mind (BTW: one of the current disadvantages is also the
fact that each indentation level _must_ be 4 spaces, otherwise you'll
get an error). Please be aware that this doesn't make almost any sense,
it just shows a couple of things I think could help everyone a lot.
file testcase.cfg:
# Local variables. You can use these variables only in this testcase
# file. This requires 1 more line of code in CofigParser.
[LocalVariables]
my_hyper = qemu
module = connections
[GlobalVariables]
# Could be named Defaults as these variables will be passed to all
# test in this testcase file...
uri = %(my_hyper)s:///system
# This section is not needed if the tests are named Test1.Connect and
# so on, but it is more readable a understandable for some readers
[Tests]
Test_1 = Connect
Test_2 = Disconnect
[Test.Connect]
Module = $(connections)s
Testcase = connect
# if not specified, this could default to Test.name.Params
Params = SomethingElse
[SomethingElse]
# ...unless they are overwritten with some others
uri = %(my_hyper)s:///session
[Test.Connect]
Module = $(connections)s
Testcase = disconnect
# No parameters here (none needed)
And then you will have two tests that look something like this (very
rough idea with lots of things I had in my mind, just to show how nice
it could look):
file tests/connections/connect.py:
def get_params(params):
# clean the
Re: [libvirt] [test-API] RFC: Stabilization of libvirt-test-API
On 03/29/2012 04:20 PM, Guannan Ren wrote:
On 03/29/2012 08:14 PM, Martin Kletzander wrote:
- fix hard-coded options into real options (e.g. commit 65449e)
- fix some env_* and util* code (functions duplicated with different
behavior)
- fix or remove harmful and pointless code (at this point, when
creating domain on remote machine, be prepared for the test to fail with
any other user then root and with root, have backup of both local and
remote '/root/.ssh' directories as the contents will be erased!)
- fix method names for the {connect,domain,etc.}API (get_host_name vs.
lookupByUUID etc.)
The optional things:
- get rid of classes in lib and make just few utility functions
covering *only* the methods that do something else than call the same
method in underlying class from the libvirt module.
- get rid of the new exception (I don't see any other difference than
in the name, which can make a difference in except: clause, but it's
converted everywhere)
the above should be easy to fix to cleanup.
I can do it.
- be able to share variables between tests (connection object and
anything else)
This belongs to new feature, we better consider it later.
No problem with that.
- introduce new config file for tests (.ini format, can be parsed by
ConfigParser, same as env.cfg, define variables used throughout the test
specifications
Please list out some critical cause, why?
Look at the mail I sent to Osier, I think it could ease the look of it
pretty much.
- update the documentation
I can do it.
- use some python code style (PEP-8?), make use of True/False, None
we used pylint to review it, It is fine.(maybe could be better)
- eliminate duplicated (and x-plicated) code (append_path in all the
files, etc.)
easy to do.
Guannan Ren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Thanks for your opinion, The only thing I'm thinking about is if it is
worth doing in the current code, but as Dave said, the main thing is the
deadline now and it's the end of April. After that, when I'm done with
school, I can put something together in my free time and present it,
maybe you'll like it, but there is no time for that right now.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Prevent crash of libvirtd without channel target name
* src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): avoid crashing
libvirtd due
to derefing a NULL pointer.
For details, please see bug:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=808371
Signed-off-by: Alex Jia a...@redhat.com
---
src/conf/domain_conf.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..4caef6f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9597,10 +9597,10 @@ static bool
virDomainChannelDefCheckABIStability(virDomainChrDefPtr src,
switch (src-targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
-if (STRNEQ(src-target.name, dst-target.name)) {
+if (STRNEQ_NULLABLE(src-target.name, dst-target.name)) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Target channel name %s does not match
source %s),
- dst-target.name, src-target.name);
+ NULLSTR(dst-target.name),
NULLSTR(src-target.name));
goto cleanup;
}
break;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Prevent crash of libvirtd without channel target name
On 30.03.2012 11:44, Alex Jia wrote: * src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): avoid crashing libvirtd due to derefing a NULL pointer. For details, please see bug: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=808371 Signed-off-by: Alex Jia a...@redhat.com --- src/conf/domain_conf.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Prevent crash of libvirtd without channel target name
Thanks and pushed now. - Original Message - From: Michal Privoznik mpriv...@redhat.com To: Alex Jia a...@redhat.com Cc: libvir-list@redhat.com Sent: Friday, March 30, 2012 6:02:21 PM Subject: Re: [libvirt] [PATCH] conf: Prevent crash of libvirtd without channel target name On 30.03.2012 11:44, Alex Jia wrote: * src/conf/domain_conf.c (virDomainChannelDefCheckABIStability): avoid crashing libvirtd due to derefing a NULL pointer. For details, please see bug: RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=808371 Signed-off-by: Alex Jia a...@redhat.com --- src/conf/domain_conf.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On Fri, Mar 30, 2012 at 03:13:31AM -0400, Laine Stump wrote:
On 03/29/2012 05:52 PM, Daniel P. Berrange wrote:
On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote:
+if (oldbridge
+virNetDevBridgeRemovePort(oldbridge, olddev-ifname) 0) {
+return -1;
+}
+if (virNetDevBridgeAddPort(newbridge, olddev-ifname) 0) {
+if (virNetDevBridgeAddPort(oldbridge, olddev-ifname) 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to recover former state by adding
port
+ to bridge %s), oldbridge);
+}
+return -1;
+}
I think you need to emit 2 audit notifications here, one for the bridge
being removed and one for the bridge being added.
That does sound like a good idea, but the current virDomainAuditNet()
function only reports MAC address, and virDomainAuditNetDevice() only
reports /dev/net/tun - neither of them gives any information about the
name of tap device or which bridge it is being attached to.
Right now, here are the audit messages that are logged when I do a full
device detach/attach of a network device:
type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=detach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=52:54:00:00:01:81 new-net=?: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/net/tun rdev=0A:C8:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/vhost-net rdev=0A:EE:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=attach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=? new-net=52:54:00:00:01:81: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
It does a good job of telling me the MAC address that's going to be used
by the domain, but nothing about how it's connected to the network.
Hmm, this seems flawed to me. The intent of the auditing code we added
was to provide information about any host resource that the VM is
associated with. So I'm really surprised we're not providing any info
about the host network interface. I suspect this should be fixed.
If we're staying within the current boundaries of reporting, is there
really value in logging a pair of messages that are ultimately just
telling us that the same mac address was detached then immediately
reattached, but not saying anything about what it was connected to?
Alternately, if we're going to start reporting about changes in network
connection, shouldn't we also be reporting what those connections are to
begin with? (I think that's a change in scope of what's being audited,
and requires a separate patch if we decide we want to do that).
I think we should still audit this change, even though current audit
rules appear broken.
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] [test-API] RFC: Stabilization of libvirt-test-API
On 03/30/2012 05:33 PM, Martin Kletzander wrote:
To answer two of your questions at once, I'll show you an example of
what I had in mind (BTW: one of the current disadvantages is also the
fact that each indentation level _must_ be 4 spaces, otherwise you'll
get an error). Please be aware that this doesn't make almost any sense,
it just shows a couple of things I think could help everyone a lot.
file testcase.cfg:
# Local variables. You can use these variables only in this testcase
# file. This requires 1 more line of code in CofigParser.
[LocalVariables]
my_hyper = qemu
module = connections
[GlobalVariables]
# Could be named Defaults as these variables will be passed to all
# test in this testcase file...
uri = %(my_hyper)s:///system
# This section is not needed if the tests are named Test1.Connect and
# so on, but it is more readable a understandable for some readers
[Tests]
Test_1 = Connect
Test_2 = Disconnect
[Test.Connect]
Module = $(connections)s
Testcase = connect
# if not specified, this could default to Test.name.Params
Params = SomethingElse
[SomethingElse]
# ...unless they are overwritten with some others
uri = %(my_hyper)s:///session
[Test.Connect]
Module = $(connections)s
Testcase = disconnect
# No parameters here (none needed)
And then you will have two tests that look something like this (very
rough idea with lots of things I had in my mind, just to show how nice
it could look):
file tests/connections/connect.py:
def get_params(params):
# clean the parameters, put defaults for undefined ones, etc.
return params
# this means that if needed, this test will create/update these
# variables in the shares (will get to that in a few lines)
provides = ('connection', 'uri')
def run(logger, test_params, shares):
# no need to test the return code, we can raise an exception that
# will be caught outside of the test and reported through logger
params_cleaned = get_params(test_params)
# shares would be object that takes care of the values shared
# between tests, check for dependencies, etc.
shares.update('uri', conn)
logger.debug('using uri: %s' % params_cleaned['uri'])
# again, no need to check for an exception
conn = libvirt.open(params_cleaned['uri'])
# and for example it could return the values that should be
# provided in shares
return { 'connection' : conn }
file tests/connections/disconnect.py:
def get_params(params):
# clean the parameters, put defaults for undefined ones, etc.
return params
# this can be either here or evaluated when the test is trying to get
# the value from the shares object
requires = ('connection',)
def run(logger, test_params, shares):
params_cleaned = get_params(test_params)
conn = shares.get('connection')
logger.info('disconnecting from uri: %s' % conn.getURI())
conn.close()
And this would be the two tests with test case that tries to connect and
disconnect. All the errors are caught in the test runner, if some tests
depend on exception that is raised in underlying libvirt, they can catch
it themselves without propagating it up (the whole point of exceptions),
etc.
The .ini fomat is often used as data storage, that is perfect
for config file.
The testcase config currently used by test-API is the default
testcase writing format in upstream autotest that supported by
qemu.
Guannan Ren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
With latest gnulib we are checking even the lowest level functions
whether they check flags. Moreover, we are shadowing the real error
on system without TUNSETIFF support.
---
cfg.mk |2 +-
src/util/virnetdevtap.c |5 +
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index c3de533..d44c8d2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -741,7 +741,7 @@ exclude_file_name_regexp--sc_avoid_write = \
exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
-exclude_file_name_regexp--sc_flags_usage = ^docs/
+exclude_file_name_regexp--sc_flags_usage = ^(docs/|src/util/virnetdevtap\.c$$)
exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
^src/rpc/gendispatch\.pl$$
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 717b6ac..54f76bd 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -129,14 +129,12 @@ virNetDevProbeVnetHdr(int tapfd)
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags)
+ unsigned int flags ATTRIBUTE_UNUSED)
{
int fd;
struct ifreq ifr;
int ret = -1;
-virCheckFlags(VIR_NETDEV_TAP_CREATE_VNET_HDR, -1);
-
if ((fd = open(/dev/net/tun, O_RDWR)) 0) {
virReportSystemError(errno, %s,
_(Unable to open /dev/net/tun, is tun module
loaded?));
@@ -241,7 +239,6 @@ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
int *tapfd ATTRIBUTE_UNUSED,
unsigned int flags)
{
-virCheckFlags(0, -1);
virReportSystemError(ENOSYS, %s,
_(Unable to create TAP devices on this platform));
return -1;
--
1.7.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libvirt:qemu: fix max_balloon less than cur_balloon
In qemuDomainGetXMLDesc, if cur_balloon is bigger than max_balloon,
max_balloon should be updated too, otherwise the currentMemory is
bigger than memory in dumped xml, which will produce invalid
checkpoint by virsh-save.
The bug appears like this below:
$virsh save vm-num foo.ckp
$virsh restore foo.ckp
error: XML error: current memory '824320k' exceeds maximum '824288k'
Signed-off-by: Zhou Peng ailvpen...@gmail.com
---
src/qemu/qemu_driver.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0d3b0bd..96d3219 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4485,7 +4485,13 @@ endjob:
if (err 0)
goto cleanup;
if (err 0)
+{
vm-def-mem.cur_balloon = balloon;
+if (balloon vm-def-mem.max_balloon)
+{
+vm-def-mem.max_balloon = balloon;
+}
+}
/* err == 0 indicates no balloon support, so ignore it */
}
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
On 03/30/2012 05:49 AM, Michal Privoznik wrote: With latest gnulib we are checking even the lowest level functions whether they check flags. Moreover, we are shadowing the real error on system without TUNSETIFF support. --- cfg.mk |2 +- src/util/virnetdevtap.c |5 + 2 files changed, 2 insertions(+), 5 deletions(-) @@ -241,7 +239,6 @@ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, int *tapfd ATTRIBUTE_UNUSED, unsigned int flags) Missing the ATTRIBUTE_UNUSED here. ACK with that fix. -- Eric Blake ebl...@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] Refactor the libvirt RPM daemon pieces
From: Daniel P. Berrange berra...@redhat.com NB: this is in response to a Fedora 17 beta blocker bug. Currently gnome-boxes depends on 'libvirt' which pulls in the default virtual network, which kills networking if you install Fedora 17 inside a KVM guest. There are a number of flaws with our packaging of the libvirtd daemon: - Installing 'libvirt' does not install 'qemu-kvm' or 'xen' etc which are required to actually run the hypervisor in question - Installing 'libvirt' pulls in the default configuration files which may not be wanted cause problems if installed inside a guest - It is not possible to explicitly required all the peices required to manage a specific hypervisor This change takes the 'libvirt' RPM and and changes it thus - libvirt: just a virtual package with dep on libvirt-daemon and libvirt-daemon-configs - libvirt-daemon: the libvirt daemon and related pieces - libvirt-daemon-configs: the default network filter configs - libvirt-docs: the website HTML We then introduce some more virtual (empty) packages - libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu' - libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm' - libvirt-daemon-lxc: Deps on libvirt-daemon - libvirt-daemon-uml: Deps on libvirt-daemon - libvirt-daemon-xen: Deps on libvirt-daemon 'xen' - libvirt-qemu: Deps on libvirt-daemon-qemu libvirt-daemon-configs - libvirt-kvm: Deps on libvirt-daemon-kvm libvirt-daemon-configs - libvirt-lxc: Deps on libvirt-daemon-lxc libvirt-daemon-configs - libvirt-uml: Deps on libvirt-daemon-uml libvirt-daemon-configs - libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-configs My intent in the future is to turn on the driver modules by default, at which time 'libvirt-daemon' will cease to include any specific drivers, instead we'll get libvirt-daemon-driver- packages for each driver. The libvirt-daemon-XXX packages will then pull in each driver that they require. It is recommended that applications required a locally installed libvirtd daemon, use either 'Requires: libvirt-daemon-' or 'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon or 'Requires: libvirt' * libvirt.spec.in: Refactor RPMs * docs/packaging.html.in, docs/sitemap.html.in: Document new RPM split rationale --- docs/packaging.html.in | 100 ++ docs/sitemap.html.in |4 + libvirt.spec.in| 474 +++- 3 files changed, 456 insertions(+), 122 deletions(-) create mode 100644 docs/packaging.html.in diff --git a/docs/packaging.html.in b/docs/packaging.html.in new file mode 100644 index 000..2e4abf7 --- /dev/null +++ b/docs/packaging.html.in @@ -0,0 +1,100 @@ +?xml version=1.0? +html + body +h1Distribution packaging/h1 + +ul id=toc/ul + +p + This page describes the rationale behind the libvirt distribution +packaging in RPM format. The RPM specfile provided with libvirt targets +all Fedora and RHEL releases. It is split up into a number of sub-RPMs +in order to facilitate minimal installations, targetting specific +feature sets. +/p + +h2Real packages/h2 + +p + The so called real packages provide the actual file payloads + related to libvirt. If very specific / targetted functionality + is required, then applications can depend on one or more of these + real packages. +/p + +dl + dtlibvirt-client/dt + ddThis package provides the main libvirt.so library along with the virsh command line tool. +If a C based application only wants to be able to manage remote hypervisors, this is all +that they need depend on/dd + dtlibvirt-devel/dt + ddThis package provides the header files and libraries required to compile and link C +applications using libvirt/dd + dtlibvirt-python/dt + ddThis package provides the Python binding to the C libraries. It will pull in the libvirt-client +RPM. If a Python application only wants to be able to manage remote hypervisors, this is all that +they need depend on/dd + dtlibvirt-daemon/dt + ddThis package provides server side libvirtd daemon, which is required in order to +manage any stateful hypervisors (currently QEMU, KVM, Xen, LXC and UML)./dd + dtlibvirt-daemon-configs/dt + ddThis package provides the standard configuration files for setting up a NAT +based network, and network filter rules for ensuring clean VM traffic./dd +/dl + +h2Virtual packages/h2 + +p + The virtual packages provide convenient targets for application dependencies to + pull in functionality related to specific hypervisors. Since the packaging of + the codelibvirt-daemon/code RPM is expected to change in the future to split + each hypervisor driver out into a separate RPM, applications are strongly + recommended to depend on one of the following virtual packages, instead of + depending
Re: [libvirt] [PATCH] virnetdevtap: Don't check for flags in virNetDevTapCreateFlags
On 30.03.2012 15:04, Eric Blake wrote: On 03/30/2012 05:49 AM, Michal Privoznik wrote: With latest gnulib we are checking even the lowest level functions whether they check flags. Moreover, we are shadowing the real error on system without TUNSETIFF support. --- cfg.mk |2 +- src/util/virnetdevtap.c |5 + 2 files changed, 2 insertions(+), 5 deletions(-) @@ -241,7 +239,6 @@ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, int *tapfd ATTRIBUTE_UNUSED, unsigned int flags) Missing the ATTRIBUTE_UNUSED here. ACK with that fix. Yeah, my in brain git-revert has some defects :) Pushed, thanks. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix deadlock when qemu cannot start
Quoting Daniel Veillard (veill...@redhat.com):
On Fri, Mar 16, 2012 at 02:37:42PM +0800, Wen Congyang wrote:
When qemu cannot start, we may call qemuProcessStop() twice.
We have check whether the vm is running at the beginning of
qemuProcessStop() to avoid libvirt deadlock. We call
qemuProcessStop() with driver and vm locked. It seems that
we can avoid libvirt deadlock. But unfortunately we may
unlock driver and vm in the function qemuProcessKill() while
vm-def-id is not -1. So qemuProcessStop() will be run twice,
and monitor will be freed unexpectedly. So we should set
vm-def-id to -1 at the beginning of qemuProcessStop().
Oh, uh, Huh. This seems like it could be responsible for what I was
reporting a few days ago (*1). I spent most of yesterday trying to
track it down, only to finally realize that the QemuProcessStart would
silently die at various places. So i was getting ready to send an email
postulating that what's happening is that a virsh list starts, then
a virsh start starts, and when the virsh list ends it somehow causes
the virsh start to be killed.
I'll test and see if this patch fixes it.
thanks,
-serge
*1: If I create 4 vms and do
for i in `seq 1 4`; do virsh start o$i /tmp/o$i 21 ; done; for i in `seq 1
4`; do virsh list /dev/null 21; done; sleep 1; virsh list
most of the time at least one vm won't start.
---
src/qemu/qemu_process.c | 20 ++--
src/qemu/qemu_process.h |2 ++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0af3751..44814df 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3633,10 +3633,11 @@ qemuProcessKill(struct qemud_driver *driver,
VIR_DEBUG(vm=%s pid=%d flags=%x,
vm-def-name, vm-pid, flags);
-if (!virDomainObjIsActive(vm)) {
-VIR_DEBUG(VM '%s' not active, vm-def-name);
-return 0;
-}
+if (!(flags VIR_QEMU_PROCESS_KILL_NOCHECK))
+if (!virDomainObjIsActive(vm)) {
+VIR_DEBUG(VM '%s' not active, vm-def-name);
+return 0;
+}
/* This loop sends SIGTERM (or SIGKILL if flags has
* VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
@@ -3739,6 +3740,13 @@ void qemuProcessStop(struct qemud_driver *driver,
return;
}
+/*
+ * We may unlock driver and vm in qemuProcessKill(), so the other
thread
+ * can lock driver and vm, and then call qemuProcessStop(). So we
should
+ * set vm-def-id to -1 here to avoid qemuProcessStop() called twice.
+ */
+vm-def-id = -1;
+
if ((logfile = qemuDomainCreateLog(driver, vm, true)) 0) {
/* To not break the normal domain shutdown process, skip the
* timestamp log writing if failed on opening log file. */
@@ -3801,7 +3809,8 @@ void qemuProcessStop(struct qemud_driver *driver,
}
/* shut it off for sure */
-ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
+ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE|
+
VIR_QEMU_PROCESS_KILL_NOCHECK));
/* Stop autodestroy in case guest is restarted */
qemuProcessAutoDestroyRemove(driver, vm);
@@ -3892,7 +3901,6 @@ retry:
vm-taint = 0;
vm-pid = -1;
-vm-def-id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
VIR_FREE(priv-vcpupids);
priv-nvcpupids = 0;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 761db6f..891f7a5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -72,6 +72,8 @@ int qemuProcessAttach(virConnectPtr conn,
typedef enum {
VIR_QEMU_PROCESS_KILL_FORCE = 1 0,
VIR_QEMU_PROCESS_KILL_NOWAIT = 1 1,
+ VIR_QEMU_PROCESS_KILL_NOCHECK = 1 2, /* donot check whether the vm is
+ running */
} virQemuProcessKillMode;
int qemuProcessKill(struct qemud_driver *driver,
Hi Wen,
sorry for the delay in reviewing the patch. I think I understand the
issue, and removing the pid and using an extra flag to qemuProcessKill
to avoid the race sounds reasonable. I rebased the patch a bit, made
some cosmetic changes especially on the comments and pushed it so it is
included in rc2 for more testing,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu_agent: Issue guest-sync prior to every command
On Fri, Mar 16, 2012 at 17:35:09 +0100, Michal Privoznik wrote: If we issue guest command and GA is not running, the issuing thread will block endlessly. We can check for GA presence by issuing guest-sync with unique ID (timestamp). We don't want to issue real command as even if GA is not running, once it is started, it process all commands written to GA socket. --- diff to v1: - don't keep list of issued IDs because it's pointless Some background on this: I've intended to switch to new guest-sync-delimited and use older guest-sync for older GA. However, since we don't use stream base implementation but use new line as delimiter for GA responses we don't need GA to issue sentinel byte 0xFF for us: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol Moreover, since we are using guest-sync just for detecting GA, it is not necessary to use sentinel byte at all: http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03278.html src/qemu/qemu_agent.c | 134 ++-- 1 files changed, 128 insertions(+), 6 deletions(-) ACK, but don't forget to run make syntax-check before pushing to fix your violation of recently added sizeof() rule. Anyway, there is a small chance the guest-agent will crash after sending us a response to guest-sync but before it can process the real command. Current code will just hang waiting for the reply. We were discussing this issue with Michal and came up with a possible solution: - use timeouts for all ga commands - any code sending ga command has to register a callback which will be used in case the command takes longer than timeout; in that case the caller already exited with error and the callback will make sure to undo any changes the command (which we thought was never acted upon) made; e.g., it will unfreeze guest's filesystem after we didn't get a reply to freeze in time - when a new command is about to be issued, guest-sync is called (already done) and getting reply to it removes any callbacks since we know it won't be processed - anytime the callback is present, we may report that guest agent is not responding to us using domcontrol (if possible) or using a new api Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On 03/30/2012 06:25 AM, Daniel P. Berrange wrote:
On Fri, Mar 30, 2012 at 03:13:31AM -0400, Laine Stump wrote:
On 03/29/2012 05:52 PM, Daniel P. Berrange wrote:
On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote:
+if (oldbridge
+virNetDevBridgeRemovePort(oldbridge, olddev-ifname) 0) {
+return -1;
+}
+if (virNetDevBridgeAddPort(newbridge, olddev-ifname) 0) {
+if (virNetDevBridgeAddPort(oldbridge, olddev-ifname) 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to recover former state by adding
port
+ to bridge %s), oldbridge);
+}
+return -1;
+}
I think you need to emit 2 audit notifications here, one for the bridge
being removed and one for the bridge being added.
That does sound like a good idea, but the current virDomainAuditNet()
function only reports MAC address, and virDomainAuditNetDevice() only
reports /dev/net/tun - neither of them gives any information about the
name of tap device or which bridge it is being attached to.
Right now, here are the audit messages that are logged when I do a full
device detach/attach of a network device:
type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=detach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=52:54:00:00:01:81 new-net=?: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/net/tun rdev=0A:C8:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/vhost-net rdev=0A:EE:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=attach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=? new-net=52:54:00:00:01:81: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
It does a good job of telling me the MAC address that's going to be used
by the domain, but nothing about how it's connected to the network.
Hmm, this seems flawed to me. The intent of the auditing code we added
was to provide information about any host resource that the VM is
associated with. So I'm really surprised we're not providing any info
about the host network interface. I suspect this should be fixed.
If we're staying within the current boundaries of reporting, is there
really value in logging a pair of messages that are ultimately just
telling us that the same mac address was detached then immediately
reattached, but not saying anything about what it was connected to?
Alternately, if we're going to start reporting about changes in network
connection, shouldn't we also be reporting what those connections are to
begin with? (I think that's a change in scope of what's being audited,
and requires a separate patch if we decide we want to do that).
I think we should still audit this change, even though current audit
rules appear broken.
Okay. Would the patch I've attached here be adequate? If so, I'll squash
it into the rest of the patch.
Beyond that, to fix the problem of general inadequacy in the current
audits, would it be enough to add tap device and bridge names to the
current attach and detach log messages? Or is the audit message format
very strict, and we would need to do a separate log message for tap
device and for bridge device?
From 3b277f2c2138b98145d6b5e18c844be5d43b8b29 Mon Sep 17 00:00:00 2001
From: Laine Stump la...@laine.org
Date: Fri, 30 Mar 2012 10:13:37 -0400
Subject: [PATCH] qemu: add audit logs when switching bridges
This adds in a standard audit log for detaching and attaching a
network device when the bridge being used is changed.
The discussion about this led to the idea that the audit logs being
generated are insufficient, since they don't say anything about which
tap device is used, nor about which bridge it is attached to, but that
should be fixed by a separate patch, and this gets the current patch
properly wired into the infrastructure.
---
src/qemu/qemu_hotplug.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 66837c4..7699e9d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1194,7 +1194,8
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On Fri, Mar 30, 2012 at 10:30:01AM -0400, Laine Stump wrote:
On 03/30/2012 06:25 AM, Daniel P. Berrange wrote:
On Fri, Mar 30, 2012 at 03:13:31AM -0400, Laine Stump wrote:
On 03/29/2012 05:52 PM, Daniel P. Berrange wrote:
On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote:
+if (oldbridge
+virNetDevBridgeRemovePort(oldbridge, olddev-ifname) 0) {
+return -1;
+}
+if (virNetDevBridgeAddPort(newbridge, olddev-ifname) 0) {
+if (virNetDevBridgeAddPort(oldbridge, olddev-ifname) 0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+_(unable to recover former state by adding
port
+ to bridge %s), oldbridge);
+}
+return -1;
+}
I think you need to emit 2 audit notifications here, one for the bridge
being removed and one for the bridge being added.
That does sound like a good idea, but the current virDomainAuditNet()
function only reports MAC address, and virDomainAuditNetDevice() only
reports /dev/net/tun - neither of them gives any information about the
name of tap device or which bridge it is being attached to.
Right now, here are the audit messages that are logged when I do a full
device detach/attach of a network device:
type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=detach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=52:54:00:00:01:81 new-net=?: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/net/tun rdev=0A:C8:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=open vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
net=52:54:00:00:01:81 path=/dev/vhost-net rdev=0A:EE:
exe=/usr/sbin/libvirtd hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0
auid=4294967295 ses=4294967295
subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net
reason=attach vm=F14 uuid=de3aa186-be64-088e-b64a-a1a03e023ffd
old-net=? new-net=52:54:00:00:01:81: exe=/usr/sbin/libvirtd hostname=?
addr=? terminal=? res=success'
It does a good job of telling me the MAC address that's going to be used
by the domain, but nothing about how it's connected to the network.
Hmm, this seems flawed to me. The intent of the auditing code we added
was to provide information about any host resource that the VM is
associated with. So I'm really surprised we're not providing any info
about the host network interface. I suspect this should be fixed.
If we're staying within the current boundaries of reporting, is there
really value in logging a pair of messages that are ultimately just
telling us that the same mac address was detached then immediately
reattached, but not saying anything about what it was connected to?
Alternately, if we're going to start reporting about changes in network
connection, shouldn't we also be reporting what those connections are to
begin with? (I think that's a change in scope of what's being audited,
and requires a separate patch if we decide we want to do that).
I think we should still audit this change, even though current audit
rules appear broken.
Okay. Would the patch I've attached here be adequate? If so, I'll squash
it into the rest of the patch.
Beyond that, to fix the problem of general inadequacy in the current
audits, would it be enough to add tap device and bridge names to the
current attach and detach log messages? Or is the audit message format
very strict, and we would need to do a separate log message for tap
device and for bridge device?
From 3b277f2c2138b98145d6b5e18c844be5d43b8b29 Mon Sep 17 00:00:00 2001
From: Laine Stump la...@laine.org
Date: Fri, 30 Mar 2012 10:13:37 -0400
Subject: [PATCH] qemu: add audit logs when switching bridges
This adds in a standard audit log for detaching and attaching a
network device when the bridge being used is changed.
The discussion about this led to the idea that the audit logs being
generated are insufficient, since they don't say anything about which
tap device is used, nor about which bridge it is attached to, but that
should be fixed by a separate patch, and this gets the current patch
properly wired into the infrastructure.
---
src/qemu/qemu_hotplug.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git
Re: [libvirt] [PATCH] qemu: set default name for SPICE agent channel when generating command
On 30.03.2012 09:50, Laine Stump wrote:
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).
This patch reverts commit b0e2bb33 and replaces it with the alternate
implementation of simply inserting the default value in the
appropriate place on the qemu commandline when no value is provided.
---
(Playing the devil's advocate on my own patch for a moment - one
advantage of Christophe's method of setting the default is that if we
use that object somewhere else in libvirt's code in the future, the
value will be set even if the XML left it unset, but with my method we
will have to check for a NULL name and replace it with the default
value anywhere we want to use it. So I won't say that either method is
definitely the proper way to go, but will just present this
alternative and see if someone else has an even stronger opinion than
me :-)
(BTW, I think I've decided while typing this message that, if we
decide to change from the original method of setting default to this
new method, the change should be pushed as two separate patches - one
reverting the original, and another adding the new. It's too close to
morning for me to take the time to do that right now, though.)
src/conf/domain_conf.c |7 ---
src/qemu/qemu_command.c |6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..ea558bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5248,13 +5248,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
goto error;
} else {
def-source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT;
-if (!def-target.name) {
-def-target.name = strdup(com.redhat.spice.0);
-if (!def-target.name) {
-virReportOOMError();
-goto error;
-}
-}
}
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..50b1e6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3457,9 +3457,9 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) {
virBufferAsprintf(buf, ,chardev=char%s,id=%s,
dev-info.alias, dev-info.alias);
-if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-dev-target.name) {
-virBufferAsprintf(buf, ,name=%s, dev-target.name);
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) {
+virBufferAsprintf(buf, ,name=%s, dev-target.name
+ ? dev-target.name : com.redhat.spice.0);
}
} else {
virBufferAsprintf(buf, ,id=%s, dev-info.alias);
ACK
Maybe it's worth squashing this in:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..3a14727 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3444,8 +3444,7 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr
dev,
if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
dev-source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC
-dev-target.name
-STRNEQ(dev-target.name, com.redhat.spice.0)) {
+STRNEQ_NULLABLE(dev-target.name, com.redhat.spice.0)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported spicevmc target name '%s'),
dev-target.name);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On Fri, Mar 30, 2012 at 02:22:01PM +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com NB: this is in response to a Fedora 17 beta blocker bug. Currently gnome-boxes depends on 'libvirt' which pulls in the default virtual network, which kills networking if you install Fedora 17 inside a KVM guest. There are a number of flaws with our packaging of the libvirtd daemon: - Installing 'libvirt' does not install 'qemu-kvm' or 'xen' etc which are required to actually run the hypervisor in question - Installing 'libvirt' pulls in the default configuration files which may not be wanted cause problems if installed inside a guest - It is not possible to explicitly required all the peices required to manage a specific hypervisor This change takes the 'libvirt' RPM and and changes it thus - libvirt: just a virtual package with dep on libvirt-daemon and libvirt-daemon-configs - libvirt-daemon: the libvirt daemon and related pieces - libvirt-daemon-configs: the default network filter configs - libvirt-docs: the website HTML We then introduce some more virtual (empty) packages - libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu' - libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm' - libvirt-daemon-lxc: Deps on libvirt-daemon - libvirt-daemon-uml: Deps on libvirt-daemon - libvirt-daemon-xen: Deps on libvirt-daemon 'xen' - libvirt-qemu: Deps on libvirt-daemon-qemu libvirt-daemon-configs - libvirt-kvm: Deps on libvirt-daemon-kvm libvirt-daemon-configs - libvirt-lxc: Deps on libvirt-daemon-lxc libvirt-daemon-configs - libvirt-uml: Deps on libvirt-daemon-uml libvirt-daemon-configs - libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-configs My intent in the future is to turn on the driver modules by default, at which time 'libvirt-daemon' will cease to include any specific drivers, instead we'll get libvirt-daemon-driver- packages for each driver. The libvirt-daemon-XXX packages will then pull in each driver that they require. It is recommended that applications required a locally installed libvirtd daemon, use either 'Requires: libvirt-daemon-' or 'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon or 'Requires: libvirt' I'm not an RPM wizard, so I cannot evaluate the changes, but ACK to the design, I think it's a great change. Dave * libvirt.spec.in: Refactor RPMs * docs/packaging.html.in, docs/sitemap.html.in: Document new RPM split rationale --- docs/packaging.html.in | 100 ++ docs/sitemap.html.in |4 + libvirt.spec.in| 474 +++- 3 files changed, 456 insertions(+), 122 deletions(-) create mode 100644 docs/packaging.html.in diff --git a/docs/packaging.html.in b/docs/packaging.html.in new file mode 100644 index 000..2e4abf7 --- /dev/null +++ b/docs/packaging.html.in @@ -0,0 +1,100 @@ +?xml version=1.0? +html + body +h1Distribution packaging/h1 + +ul id=toc/ul + +p + This page describes the rationale behind the libvirt distribution +packaging in RPM format. The RPM specfile provided with libvirt targets +all Fedora and RHEL releases. It is split up into a number of sub-RPMs +in order to facilitate minimal installations, targetting specific +feature sets. +/p + +h2Real packages/h2 + +p + The so called real packages provide the actual file payloads + related to libvirt. If very specific / targetted functionality + is required, then applications can depend on one or more of these + real packages. +/p + +dl + dtlibvirt-client/dt + ddThis package provides the main libvirt.so library along with the virsh command line tool. +If a C based application only wants to be able to manage remote hypervisors, this is all +that they need depend on/dd + dtlibvirt-devel/dt + ddThis package provides the header files and libraries required to compile and link C +applications using libvirt/dd + dtlibvirt-python/dt + ddThis package provides the Python binding to the C libraries. It will pull in the libvirt-client +RPM. If a Python application only wants to be able to manage remote hypervisors, this is all that +they need depend on/dd + dtlibvirt-daemon/dt + ddThis package provides server side libvirtd daemon, which is required in order to +manage any stateful hypervisors (currently QEMU, KVM, Xen, LXC and UML)./dd + dtlibvirt-daemon-configs/dt + ddThis package provides the standard configuration files for setting up a NAT +based network, and network filter rules for ensuring clean VM traffic./dd +/dl + +h2Virtual packages/h2 + +p + The virtual packages provide convenient targets for application dependencies to + pull in functionality related to
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On Fri, Mar 30, 2012 at 02:22:01PM +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com NB: this is in response to a Fedora 17 beta blocker bug. Currently gnome-boxes depends on 'libvirt' which pulls in the default virtual network, which kills networking if you install Fedora 17 inside a KVM guest. There are a number of flaws with our packaging of the libvirtd daemon: - Installing 'libvirt' does not install 'qemu-kvm' or 'xen' etc which are required to actually run the hypervisor in question - Installing 'libvirt' pulls in the default configuration files which may not be wanted cause problems if installed inside a guest - It is not possible to explicitly required all the peices required to manage a specific hypervisor This change takes the 'libvirt' RPM and and changes it thus - libvirt: just a virtual package with dep on libvirt-daemon and libvirt-daemon-configs - libvirt-daemon: the libvirt daemon and related pieces - libvirt-daemon-configs: the default network filter configs It occurs to me that we perhaps ought to split the configs into two pieces too. The Xen and UML drivers only support the network driver, so they don't need the network filter configs The OpenStack NOVA project does not want the default NAT network, but it does want the network filter configs. So I'm thinking instead of libvirt-daemon-configs, create: - libvirt-daemon-config-network - libvirt-daemon-config-nwfilter Regards, 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] [ANNOUNCE] libvirt-glib 0.0.7 release
am pleased to announce that a new release of the libvirt-glib package, version 0.0.7 is now available from ftp://libvirt.org/libvirt/glib/ The packages are GPG signed with Key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R) New in this release: - Fix typo in filesystem access mode constant - Remove incorrect encoding of XML attributes - Add support for USB redirection devices - Add support for SPICE agent device - Fix typo in channel device target constant - Make all string getters return a const string - Keep list of devices sorted in original XML order libvirt-glib comprises three distinct libraries: - libvirt-glib- Integrate with the GLib event loop and error handling - libvirt-gconfig - Representation of libvirt XML documents as GObjects - libvirt-gobject - Mapping of libvirt APIs into the GObject type system NB: While libvirt aims to be API/ABI stable, for the first few releases, we are *NOT* guaranteeing that libvirt-glib libraries are API/ABI stable. ABI stability will only be guaranteed once the bulk of the APIs have been fleshed out and proved in non-trivial application usage. We anticipate this will be within the next 6 months in order to line up with Fedora 17. Follow up comments about libvirt-glib should be directed to the regular libvir-list redhat com development list. Thanks to all the people involved in contributing to this release. Regards, 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] fix deadlock when qemu cannot start
On Fri, Mar 30, 2012 at 08:45:35AM -0500, Serge Hallyn wrote: Quoting Daniel Veillard (veill...@redhat.com): On Fri, Mar 16, 2012 at 02:37:42PM +0800, Wen Congyang wrote: When qemu cannot start, we may call qemuProcessStop() twice. We have check whether the vm is running at the beginning of qemuProcessStop() to avoid libvirt deadlock. We call qemuProcessStop() with driver and vm locked. It seems that we can avoid libvirt deadlock. But unfortunately we may unlock driver and vm in the function qemuProcessKill() while vm-def-id is not -1. So qemuProcessStop() will be run twice, and monitor will be freed unexpectedly. So we should set vm-def-id to -1 at the beginning of qemuProcessStop(). Oh, uh, Huh. This seems like it could be responsible for what I was reporting a few days ago (*1). I spent most of yesterday trying to track it down, only to finally realize that the QemuProcessStart would silently die at various places. So i was getting ready to send an email postulating that what's happening is that a virsh list starts, then a virsh start starts, and when the virsh list ends it somehow causes the virsh start to be killed. I'll test and see if this patch fixes it. I guess I need to resurect my patch checker, tune it and make it send warning on patches not ACK'ed or reviewed automatically, or even better make it a web page to avoid boring people. http://libvirt.org/git/?p=patchchecker.git;a=summary I had that patch on my radar and though I took an awful long time to review it it's there. BTW I'm late for rc2, I will make it tomorrow I guess, sorry about it this is due to events out of my control :-\ Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] conf: allow fuzz in XML with cur balloon max
Commit 1b1402b introduced a regression. Since older libvirt versions
would silently round memory up (until the previous patch), but populated
current memory based on querying the guest, it was possible to have
current maximum by the amount of the rounding. Accept this fuzz
factor, and silently clamp current down to maximum in that case,
rather than failing to reparse XML for an existing VM.
* src/conf/domain_conf.c (virDomainDefParseXML): Don't reject
existing XML.
Based on a report by Zhou Peng.
---
src/conf/domain_conf.c | 19 +++
src/qemu/qemu_monitor_json.c |2 +-
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4caef6f..7c95920 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7776,10 +7776,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr
caps,
goto error;
if (def-mem.cur_balloon def-mem.max_balloon) {
-virDomainReportError(VIR_ERR_XML_ERROR,
- _(current memory '%lluk' exceeds maximum
'%lluk'),
- def-mem.cur_balloon, def-mem.max_balloon);
-goto error;
+/* Older libvirt could get into this situation due to
+ * rounding; if the discrepancy is less than 1MiB, we silently
+ * round down, otherwise we flag the issue. */
+if (VIR_DIV_UP(def-mem.cur_balloon, 1024)
+VIR_DIV_UP(def-mem.max_balloon, 1024)) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(current memory '%lluk' exceeds
+ maximum '%lluk'),
+ def-mem.cur_balloon, def-mem.max_balloon);
+goto error;
+} else {
+VIR_DEBUG(Truncating current %lluk to maximum %lluk,
+ def-mem.cur_balloon, def-mem.max_balloon);
+def-mem.cur_balloon = def-mem.max_balloon;
+}
} else if (def-mem.cur_balloon == 0) {
def-mem.cur_balloon = def-mem.max_balloon;
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7093e1d..6d66587 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2022,7 +2022,7 @@ int qemuMonitorJSONSetBalloon(qemuMonitorPtr mon,
{
int ret;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(balloon,
- U:value, ((unsigned
long long)newmem)*1024,
+ U:value, newmem*1024ULL,
NULL);
virJSONValuePtr reply = NULL;
if (!cmd)
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] qemu: reflect any memory rounding back to xml
If we round up a user's memory request, we should update the XML
to reflect the actual value in use by the VM, rather than giving
an artificially small value back to the user.
* src/qemu/qemu_command.c (qemuBuildNumaArgStr)
(qemuBuildCommandLine): Reflect rounding back to XML.
---
src/qemu/qemu_command.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..8f6471b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3916,8 +3916,9 @@ qemuBuildNumaArgStr(const virDomainDefPtr def,
virCommandPtr cmd)
virBufferAsprintf(buf, node,nodeid=%d, def-cpu-cells[i].cellid);
virBufferAddLit(buf, ,cpus=);
qemuBuildNumaCPUArgStr(def-cpu-cells[i].cpumask, buf);
-virBufferAsprintf(buf, mem=%d,
-VIR_DIV_UP(def-cpu-cells[i].mem, 1024));
+def-cpu-cells[i].mem = VIR_DIV_UP(def-cpu-cells[i].mem,
+1024) * 1024;
+virBufferAsprintf(buf, mem=%d, def-cpu-cells[i].mem / 1024);
if (virBufferError(buf))
goto error;
@@ -4061,10 +4062,12 @@ qemuBuildCommandLine(virConnectPtr conn,
/* Set '-m MB' based on maxmem, because the lower 'memory' limit
* is set post-startup using the balloon driver. If balloon driver
- * is not supported, then they're out of luck anyway
+ * is not supported, then they're out of luck anyway. Update the
+ * XML to reflect our rounding.
*/
virCommandAddArg(cmd, -m);
-virCommandAddArgFormat(cmd, %llu, VIR_DIV_UP(def-mem.max_balloon,
1024));
+def-mem.max_balloon = VIR_DIV_UP(def-mem.max_balloon, 1024) * 1024;
+virCommandAddArgFormat(cmd, %llu, def-mem.max_balloon / 1024);
if (def-mem.hugepage_backed) {
if (!driver-hugetlbfs_mount) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] allow for existing XML with cur max memory
NACK to Zhou's patch (it is only a bandaid trying to cover the symptom, rather than fixing the real bug that happened earlier), but I'm proposing this series in its place. Eric Blake (2): qemu: reflect any memory rounding back to xml conf: allow fuzz in XML with cur balloon max src/conf/domain_conf.c | 19 +++ src/qemu/qemu_command.c | 11 +++ src/qemu/qemu_monitor_json.c |2 +- 3 files changed, 23 insertions(+), 9 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On 03/30/2012 09:22 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com NB: this is in response to a Fedora 17 beta blocker bug. Currently gnome-boxes depends on 'libvirt' which pulls in the default virtual network, which kills networking if you install Fedora 17 inside a KVM guest. There are a number of flaws with our packaging of the libvirtd daemon: - Installing 'libvirt' does not install 'qemu-kvm' or 'xen' etc which are required to actually run the hypervisor in question - Installing 'libvirt' pulls in the default configuration files which may not be wanted cause problems if installed inside a guest - It is not possible to explicitly required all the peices required to manage a specific hypervisor This change takes the 'libvirt' RPM and and changes it thus - libvirt: just a virtual package with dep on libvirt-daemon and libvirt-daemon-configs - libvirt-daemon: the libvirt daemon and related pieces - libvirt-daemon-configs: the default network filter configs - libvirt-docs: the website HTML We then introduce some more virtual (empty) packages - libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu' - libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm' - libvirt-daemon-lxc: Deps on libvirt-daemon - libvirt-daemon-uml: Deps on libvirt-daemon - libvirt-daemon-xen: Deps on libvirt-daemon 'xen' - libvirt-qemu: Deps on libvirt-daemon-qemu libvirt-daemon-configs - libvirt-kvm: Deps on libvirt-daemon-kvm libvirt-daemon-configs - libvirt-lxc: Deps on libvirt-daemon-lxc libvirt-daemon-configs - libvirt-uml: Deps on libvirt-daemon-uml libvirt-daemon-configs - libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-configs My intent in the future is to turn on the driver modules by default, at which time 'libvirt-daemon' will cease to include any specific drivers, instead we'll get libvirt-daemon-driver- packages for each driver. The libvirt-daemon-XXX packages will then pull in each driver that they require. I like the modularity of this package setup. There is one difference with the change described in the final paragraph, though, that will cause some surprises when it's done: With the current setup, you can install the libvirt package, and it will just use/present whichever hypervisors happen to be installed on the machine. With the new setup, if you have qemu-kvm and lxc installed on the machine, then install libvirt, you won't get the libvirt driver for either of these - you'll need to explicitly install libvirt-qemu and libvirt-lxc. This may cause surprises for people who already have bootstraps that just install qemu-kvm and libvirt. Is there a way within the package system to specify something like If package X is installed, then also require package libvirt-X? It is recommended that applications required a locally installed libvirtd daemon, use either 'Requires: libvirt-daemon-' or 'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon or 'Requires: libvirt' * libvirt.spec.in: Refactor RPMs * docs/packaging.html.in, docs/sitemap.html.in: Document new RPM split rationale --- docs/packaging.html.in | 100 ++ docs/sitemap.html.in |4 + libvirt.spec.in| 474 +++- 3 files changed, 456 insertions(+), 122 deletions(-) create mode 100644 docs/packaging.html.in diff --git a/docs/packaging.html.in b/docs/packaging.html.in new file mode 100644 index 000..2e4abf7 --- /dev/null +++ b/docs/packaging.html.in @@ -0,0 +1,100 @@ +?xml version=1.0? +html + body +h1Distribution packaging/h1 + +ul id=toc/ul + +p + This page describes the rationale behind the libvirt distribution +packaging in RPM format. The RPM specfile provided with libvirt targets +all Fedora and RHEL releases. It is split up into a number of sub-RPMs +in order to facilitate minimal installations, targetting specific +feature sets. +/p + +h2Real packages/h2 + +p + The so called real packages provide the actual file payloads + related to libvirt. If very specific / targetted functionality + is required, then applications can depend on one or more of these + real packages. +/p + +dl + dtlibvirt-client/dt + ddThis package provides the main libvirt.so library along with the virsh command line tool. Several long lines. It would be nice to wrap them to 80 columns. +If a C based application only wants to be able to manage remote hypervisors, this is all +that they need depend on/dd + dtlibvirt-devel/dt + ddThis package provides the header files and libraries required to compile and link C +applications using libvirt/dd + dtlibvirt-python/dt + ddThis package provides the Python binding to the C libraries. It will pull in the libvirt-client +RPM. If a Python application only
Re: [libvirt] [PATCH v2] qemu_agent: Issue guest-sync prior to every command
On 30.03.2012 16:31, Jiri Denemark wrote: On Fri, Mar 16, 2012 at 17:35:09 +0100, Michal Privoznik wrote: If we issue guest command and GA is not running, the issuing thread will block endlessly. We can check for GA presence by issuing guest-sync with unique ID (timestamp). We don't want to issue real command as even if GA is not running, once it is started, it process all commands written to GA socket. --- diff to v1: - don't keep list of issued IDs because it's pointless Some background on this: I've intended to switch to new guest-sync-delimited and use older guest-sync for older GA. However, since we don't use stream base implementation but use new line as delimiter for GA responses we don't need GA to issue sentinel byte 0xFF for us: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol Moreover, since we are using guest-sync just for detecting GA, it is not necessary to use sentinel byte at all: http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03278.html src/qemu/qemu_agent.c | 134 ++-- 1 files changed, 128 insertions(+), 6 deletions(-) ACK, but don't forget to run make syntax-check before pushing to fix your violation of recently added sizeof() rule. Thanks pushed. Anyway, there is a small chance the guest-agent will crash after sending us a response to guest-sync but before it can process the real command. Current code will just hang waiting for the reply. We were discussing this issue with Michal and came up with a possible solution: - use timeouts for all ga commands - any code sending ga command has to register a callback which will be used in case the command takes longer than timeout; in that case the caller already exited with error and the callback will make sure to undo any changes the command (which we thought was never acted upon) made; e.g., it will unfreeze guest's filesystem after we didn't get a reply to freeze in time - when a new command is about to be issued, guest-sync is called (already done) and getting reply to it removes any callbacks since we know it won't be processed - anytime the callback is present, we may report that guest agent is not responding to us using domcontrol (if possible) or using a new api Jirka Yeah, I'll post follow up patch once we're after release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On Fri, Mar 30, 2012 at 12:13:13PM -0400, Laine Stump wrote:
On 03/30/2012 09:22 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
NB: this is in response to a Fedora 17 beta blocker bug.
Currently gnome-boxes depends on 'libvirt' which pulls
in the default virtual network, which kills networking
if you install Fedora 17 inside a KVM guest.
There are a number of flaws with our packaging of the libvirtd
daemon:
- Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
etc which are required to actually run the hypervisor in
question
- Installing 'libvirt' pulls in the default configuration
files which may not be wanted cause problems if installed
inside a guest
- It is not possible to explicitly required all the peices
required to manage a specific hypervisor
This change takes the 'libvirt' RPM and and changes it thus
- libvirt: just a virtual package with dep on libvirt-daemon and
libvirt-daemon-configs
- libvirt-daemon: the libvirt daemon and related pieces
- libvirt-daemon-configs: the default network filter configs
- libvirt-docs: the website HTML
We then introduce some more virtual (empty) packages
- libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu'
- libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm'
- libvirt-daemon-lxc: Deps on libvirt-daemon
- libvirt-daemon-uml: Deps on libvirt-daemon
- libvirt-daemon-xen: Deps on libvirt-daemon 'xen'
- libvirt-qemu: Deps on libvirt-daemon-qemu libvirt-daemon-configs
- libvirt-kvm: Deps on libvirt-daemon-kvm libvirt-daemon-configs
- libvirt-lxc: Deps on libvirt-daemon-lxc libvirt-daemon-configs
- libvirt-uml: Deps on libvirt-daemon-uml libvirt-daemon-configs
- libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-configs
My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.
I like the modularity of this package setup. There is one difference
with the change described in the final paragraph, though, that will
cause some surprises when it's done:
With the current setup, you can install the libvirt package, and it
will just use/present whichever hypervisors happen to be installed on
the machine. With the new setup, if you have qemu-kvm and lxc installed
on the machine, then install libvirt, you won't get the libvirt driver
for either of these - you'll need to explicitly install libvirt-qemu and
libvirt-lxc. This may cause surprises for people who already have
bootstraps that just install qemu-kvm and libvirt.
No, that's not the case. The behaviour of 'yum install libvirt' is
identical before after. You won't get the 'libvirt-daemon-kvm'
package installed, but that's fine, because that's an empty
virtual package.
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 67f1c33..743d43e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -52,6 +52,14 @@
%define with_libxl 0%{!?_without_libxl:%{server_drivers}}
%define with_vmware0%{!?_without_vmware:%{server_drivers}}
+%define with_qemu_tcg %{with_qemu}
qemu_tcg wasn't mentioned anywhere in the documentation or commit log
message. Was this intentional?
(Note after further reading - it looks like qemu_tcg is being used as an
easier-to-differentiate synonym for qemu (i.e. software-only qemu), is
that right?)
Yes, this is to let us deal with RHEL where we only provide
qemu-kvm, not the rest of the TCG based binaries.
+%package docs
+Summary: Documentation for libvirt library and daemon
+Group: Development/Libraries
+
+%description docs
+Copy of the libvirt website documentation
+
+%description daemon
+Server side daemon required to manage the virtualization capabilities
+of recent versions of Linux. Requires a hypervisor specific sub-RPM
+for specific drivers.
Functionally it probably makes no difference, but I think it would be
better if %description daemon was put next to %package daemon rather
than having the %package and %description of docs in between. Or is this
grouped in some other way that I missed?
Yes, that ordering is a mistake I've fixed.
+
+%package daemon-configs
+Summary: Default configuration files for the libvirtd daemon
+Group: Development/Libraries
+
+Requires: libvirt-daemon = %{version}-%{release}
+
+%description daemon-configs
+Default configuration files for setting up NAT based networking
+and guest network traffic filtering.
+%endif
+
+# XXX when we turn on driver modules, we will need to
+# create daemon-drv-XXX sub-RPMs and add them as deps
+# to all of the following daemon-XXX RPMs
+
+%if %{with_qemu_tcg}
+%package daemon-qemu
+Summary: Server side
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On 03/30/2012 12:20 PM, Daniel P. Berrange wrote: On Fri, Mar 30, 2012 at 12:13:13PM -0400, Laine Stump wrote: With the current setup, you can install the libvirt package, and it will just use/present whichever hypervisors happen to be installed on the machine. With the new setup, if you have qemu-kvm and lxc installed on the machine, then install libvirt, you won't get the libvirt driver for either of these - you'll need to explicitly install libvirt-qemu and libvirt-lxc. This may cause surprises for people who already have bootstraps that just install qemu-kvm and libvirt. No, that's not the case. The behaviour of 'yum install libvirt' is identical before after. You won't get the 'libvirt-daemon-kvm' package installed, but that's fine, because that's an empty virtual package. That will be the case with the current changes, but will it still be true even after we switch to providing a separately compiled module for each hypervisor driver? (that's actually the case I was thinking about, not the current case) I don't think there's a good way todo it in two separate steps - in fact it would likely make it even harder to ensure a correct upgrade paths. Okay. BTW, you've probably already fixed it, but I just found that packaging.html fails to validate (haven't investigated, since it's very likely you already encountered it too) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix mingw ssize_t, syntax check
We are so close to a release that we don't want to pull in a
gnulib submodule update and risk regressions, since there has
been a lot of other gnulib churn upstream. However, there are
a couple of gnulib issues that are worth fixing in isolation,
by applying local patches to gnulib.
There was an upstream gnulib bug in maint.mk that rendered most
of our syntax checks ineffective (and fixing it flushed out a
minor bug in our code):
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
There is still an upstream bug where gnulib uses the wrong type
for ssize_t on mingw; we need the fix now even though it has not
yet been accepted into gnulib:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00188.html
* gnulib/local/top/maint.mk.diff: Pick up upstream gnulib
maint.mk.
* gnulib/local/m4/ssize_t.m4.diff: Work around gnulib bug.
* src/libvirt.c: Remove unused header.
---
Yes, I know reading a diff of a diff is hard.
Any objections to this? I'd like to apply it before RC2 is built.
I am not set up to easily test if the ssize_t fix works on mingw.
I _did_ test it on Linux, including where I primed the configure
cache to intentionally treat ssize_t as missing, so I'm confident
the replacement mechanism works; I just don't know if it will
silence the warnings that Dan reported here:
https://www.redhat.com/archives/libvir-list/2012-March/msg01273.html
And we still need _part_ of that patch (the conversion from fprintf
to asprintf, in order to support %zd in the first place).
gnulib/local/m4/ssize_t.m4.diff | 34 ++
gnulib/local/top/maint.mk.diff | 32
src/libvirt.c |2 --
3 files changed, 66 insertions(+), 2 deletions(-)
create mode 100644 gnulib/local/m4/ssize_t.m4.diff
create mode 100644 gnulib/local/top/maint.mk.diff
diff --git a/gnulib/local/m4/ssize_t.m4.diff b/gnulib/local/m4/ssize_t.m4.diff
new file mode 100644
index 000..d0ae286
--- /dev/null
+++ b/gnulib/local/m4/ssize_t.m4.diff
@@ -0,0 +1,34 @@
+diff --git i/m4/ssize_t.m4 w/m4/ssize_t.m4
+index 209d64c..5ea72a1 100644
+--- i/m4/ssize_t.m4
w/m4/ssize_t.m4
+@@ -1,4 +1,4 @@
+-# ssize_t.m4 serial 5 (gettext-0.18.2)
++# ssize_t.m4 serial 6 (gettext-0.18.2)
+ dnl Copyright (C) 2001-2003, 2006, 2010-2012 Free Software Foundation, Inc.
+ dnl This file is free software; the Free Software Foundation
+ dnl gives unlimited permission to copy and/or distribute it,
+@@ -17,7 +17,21 @@ AC_DEFUN([gt_TYPE_SSIZE_T],
+ return !x;]])],
+[gt_cv_ssize_t=yes], [gt_cv_ssize_t=no])])
+ if test $gt_cv_ssize_t = no; then
+-AC_DEFINE([ssize_t], [int],
+- [Define as a signed type of the same size as size_t.])
++AC_CACHE_CHECK([for rank of size_t], [gt_cv_size_t_rank],
++ [AC_COMPILE_IFELSE(
++[AC_LANG_PROGRAM(
++ [[#include sys/types.h
++ #ifdef __cplusplus
++ extern C {
++ #endif
++int foo(unsigned long bar);
++int foo(size_t bar);
++ #ifdef __cplusplus
++ }
++ #endif
++ ]])],
++ [gt_cv_size_t_rank=long], [gt_cv_size_t_rank=int])])
++AC_DEFINE_UNQUOTED([ssize_t], [$gt_cv_size_t_rank],
++ [Define as a signed type of the same size and rank as size_t.])
+ fi
+ ])
diff --git a/gnulib/local/top/maint.mk.diff b/gnulib/local/top/maint.mk.diff
new file mode 100644
index 000..85e97ae
--- /dev/null
+++ b/gnulib/local/top/maint.mk.diff
@@ -0,0 +1,32 @@
+diff --git i/top/maint.mk w/top/maint.mk
+index 4cbd5f4..2228a37 100644
+--- i/top/maint.mk
w/top/maint.mk
+@@ -279,7 +279,7 @@ define _sc_search_regexp
+if test -n $$files; then \
+ if test -n $$prohibit; then\
+grep $$with_grep_options $(_ignore_case) -nE $$prohibit $$files \
+- | grep -vE $${exclude-^$$} \
++ | grep -vE $${exclude:-^$$}
\
+ { msg=$$halt $(_sc_say_and_exit) } || :;
\
+ else \
+grep $$with_grep_options $(_ignore_case) -LE $$require $$files \
+@@ -455,7 +455,8 @@ sc_prohibit_quotearg_without_use:
+
+ # Don't include quote.h unless you use one of its functions.
+ sc_prohibit_quote_without_use:
+- @h='quote.h' re='\quote(_n)? *\(' $(_sc_header_without_use)
++ @h='quote.h' re='\quote((_n)? *\(|_quoting_options\)' \
++$(_sc_header_without_use)
+
+ # Don't include this header unless you use one of its functions.
+ sc_prohibit_long_options_without_use:
+@@ -1332,7 +1333,7 @@ alpha beta stable: $(local-check) writable-files
$(submodule-checks)
+ $(MAKE) vc-diff-check
+ $(MAKE) news-check
+ $(MAKE) distcheck
+- $(MAKE) dist XZ_OPT=-9ev
++ $(MAKE) dist
+ $(MAKE) $(release-prep-hook)
Re: [libvirt] [PATCH] build: fix mingw ssize_t, syntax check
On Fri, Mar 30, 2012 at 10:35:41AM -0600, Eric Blake wrote:
We are so close to a release that we don't want to pull in a
gnulib submodule update and risk regressions, since there has
been a lot of other gnulib churn upstream. However, there are
a couple of gnulib issues that are worth fixing in isolation,
by applying local patches to gnulib.
There was an upstream gnulib bug in maint.mk that rendered most
of our syntax checks ineffective (and fixing it flushed out a
minor bug in our code):
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
There is still an upstream bug where gnulib uses the wrong type
for ssize_t on mingw; we need the fix now even though it has not
yet been accepted into gnulib:
https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00188.html
* gnulib/local/top/maint.mk.diff: Pick up upstream gnulib
maint.mk.
* gnulib/local/m4/ssize_t.m4.diff: Work around gnulib bug.
* src/libvirt.c: Remove unused header.
---
Yes, I know reading a diff of a diff is hard.
Any objections to this? I'd like to apply it before RC2 is built.
I am not set up to easily test if the ssize_t fix works on mingw.
I _did_ test it on Linux, including where I primed the configure
cache to intentionally treat ssize_t as missing, so I'm confident
the replacement mechanism works; I just don't know if it will
silence the warnings that Dan reported here:
https://www.redhat.com/archives/libvir-list/2012-March/msg01273.html
And we still need _part_ of that patch (the conversion from fprintf
to asprintf, in order to support %zd in the first place).
gnulib/local/m4/ssize_t.m4.diff | 34 ++
gnulib/local/top/maint.mk.diff | 32
src/libvirt.c |2 --
3 files changed, 66 insertions(+), 2 deletions(-)
create mode 100644 gnulib/local/m4/ssize_t.m4.diff
create mode 100644 gnulib/local/top/maint.mk.diff
diff --git a/gnulib/local/m4/ssize_t.m4.diff b/gnulib/local/m4/ssize_t.m4.diff
new file mode 100644
index 000..d0ae286
--- /dev/null
+++ b/gnulib/local/m4/ssize_t.m4.diff
@@ -0,0 +1,34 @@
+diff --git i/m4/ssize_t.m4 w/m4/ssize_t.m4
+index 209d64c..5ea72a1 100644
+--- i/m4/ssize_t.m4
w/m4/ssize_t.m4
+@@ -1,4 +1,4 @@
+-# ssize_t.m4 serial 5 (gettext-0.18.2)
++# ssize_t.m4 serial 6 (gettext-0.18.2)
+ dnl Copyright (C) 2001-2003, 2006, 2010-2012 Free Software Foundation, Inc.
+ dnl This file is free software; the Free Software Foundation
+ dnl gives unlimited permission to copy and/or distribute it,
+@@ -17,7 +17,21 @@ AC_DEFUN([gt_TYPE_SSIZE_T],
+ return !x;]])],
+[gt_cv_ssize_t=yes], [gt_cv_ssize_t=no])])
+ if test $gt_cv_ssize_t = no; then
+-AC_DEFINE([ssize_t], [int],
+- [Define as a signed type of the same size as size_t.])
++AC_CACHE_CHECK([for rank of size_t], [gt_cv_size_t_rank],
++ [AC_COMPILE_IFELSE(
++[AC_LANG_PROGRAM(
++ [[#include sys/types.h
++ #ifdef __cplusplus
++ extern C {
++ #endif
++int foo(unsigned long bar);
++int foo(size_t bar);
++ #ifdef __cplusplus
++ }
++ #endif
++ ]])],
++ [gt_cv_size_t_rank=long], [gt_cv_size_t_rank=int])])
++AC_DEFINE_UNQUOTED([ssize_t], [$gt_cv_size_t_rank],
++ [Define as a signed type of the same size and rank as
size_t.])
+ fi
+ ])
diff --git a/gnulib/local/top/maint.mk.diff b/gnulib/local/top/maint.mk.diff
new file mode 100644
index 000..85e97ae
--- /dev/null
+++ b/gnulib/local/top/maint.mk.diff
@@ -0,0 +1,32 @@
+diff --git i/top/maint.mk w/top/maint.mk
+index 4cbd5f4..2228a37 100644
+--- i/top/maint.mk
w/top/maint.mk
+@@ -279,7 +279,7 @@ define _sc_search_regexp
+if test -n $$files; then
\
+ if test -n $$prohibit; then \
+grep $$with_grep_options $(_ignore_case) -nE $$prohibit $$files \
+- | grep -vE $${exclude-^$$}
\
++ | grep -vE $${exclude:-^$$}
\
+ { msg=$$halt $(_sc_say_and_exit) } || :;
\
+ else \
+grep $$with_grep_options $(_ignore_case) -LE $$require $$files \
+@@ -455,7 +455,8 @@ sc_prohibit_quotearg_without_use:
+
+ # Don't include quote.h unless you use one of its functions.
+ sc_prohibit_quote_without_use:
+-@h='quote.h' re='\quote(_n)? *\(' $(_sc_header_without_use)
++@h='quote.h' re='\quote((_n)? *\(|_quoting_options\)' \
++ $(_sc_header_without_use)
+
+ # Don't include this header unless you use one of its functions.
+ sc_prohibit_long_options_without_use:
+@@ -1332,7 +1333,7 @@ alpha beta stable: $(local-check) writable-files
[libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
From: Daniel P. Berrange berra...@redhat.com
There are a number of flaws with our packaging of the libvirtd
daemon:
- Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
etc which are required to actually run the hypervisor in
question
- Installing 'libvirt' pulls in the default configuration
files which may not be wanted cause problems if installed
inside a guest
- It is not possible to explicitly required all the peices
required to manage a specific hypervisor
This change takes the 'libvirt' RPM and and changes it thus
- libvirt: just a virtual package with dep on libvirt-daemon,
libvirt-daemon-config-network libvirt-daemon-config-nwfilter
- libvirt-daemon: the libvirt daemon and related pieces
- libvirt-daemon-config-network: the default network config
- libvirt-daemon-config-nwfilter: the network filter configs
- libvirt-docs: the website HTML
We then introduce some more virtual (empty) packages
- libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu'
- libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm'
- libvirt-daemon-lxc: Deps on libvirt-daemon
- libvirt-daemon-uml: Deps on libvirt-daemon
- libvirt-daemon-xen: Deps on libvirt-daemon 'xen'
- libvirt-qemu: Deps on libvirt-daemon-qemu
libvirt-daemon-config-{network,nwfilter}
- libvirt-kvm: Deps on libvirt-daemon-kvm
libvirt-daemon-config-{network,nwfilter}
- libvirt-lxc: Deps on libvirt-daemon-lxc
libvirt-daemon-config-{network,nwfilter}
- libvirt-uml: Deps on libvirt-daemon-uml
libvirt-daemon-config-{network,nwfilter}
- libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-config-network
My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.
It is recommended that applications required a locally installed
libvirtd daemon, use either 'Requires: libvirt-daemon-' or
'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon
or 'Requires: libvirt'
* libvirt.spec.in: Refactor RPMs
* docs/packaging.html.in, docs/sitemap.html.in: Document
new RPM split rationale
---
docs/packaging.html.in | 106 ++
docs/sitemap.html.in |4 +
libvirt.spec.in| 512 +---
3 files changed, 505 insertions(+), 117 deletions(-)
create mode 100644 docs/packaging.html.in
In v2:
- Split libvirt-daemon-configs into libvirt-daemon-config-network
and libvirt-daemon-config-nwfilter
- Fix ordering of QEMU/KVM bits
- Don't install nwfilter configs if the driver is turned off
- Enable nwfilter for LXC/UML drivers explicitly, not just KVM
- Fix docs typos
- Make all %files sections conditional
Tested build on F17 and RHEL-6.3
diff --git a/docs/packaging.html.in b/docs/packaging.html.in
new file mode 100644
index 000..0adffd3
--- /dev/null
+++ b/docs/packaging.html.in
@@ -0,0 +1,106 @@
+?xml version=1.0?
+html
+ body
+h1Distribution packaging/h1
+
+ul id=toc/ul
+
+p
+ This page describes the rationale behind the libvirt distribution
+packaging in RPM format. The RPM specfile provided with libvirt targets
+all Fedora and RHEL releases. It is split up into a number of sub-RPMs
+in order to facilitate minimal installations, targetting specific
+feature sets.
+/p
+
+h2a name=realReal packages/a/h2
+
+p
+ The so called real packages provide the actual file payloads
+ related to libvirt. If very specific / targetted functionality
+ is required, then applications can depend on one or more of these
+ real packages.
+/p
+
+dl
+ dtlibvirt-client/dt
+ ddThis package provides the main libvirt.so library along with
+the virsh command line tool. If a C based application only wants
+to be able to manage remote hypervisors, this is all that they
+need depend on/dd
+ dtlibvirt-devel/dt
+ ddThis package provides the header files and libraries required
+to compile and link C applications using libvirt/dd
+ dtlibvirt-python/dt
+ ddThis package provides the Python binding to the C libraries.
+It will pull in the libvirt-client RPM. If a Python application
+only wants to be able to manage remote hypervisors, this is all
+that they need depend on/dd
+ dtlibvirt-daemon/dt
+ ddThis package provides server side libvirtd daemon, which is
+required in order to manage any stateful hypervisors (currently
+QEMU, KVM, Xen, LXC and UML)./dd
+ dtlibvirt-daemon-config-network/dt
+ ddThis package provides the standard configuration files for
+setting up a NAT based network/dd
+ dtlibvirt-daemon-config-nwfilter/dt
+ ddThis package provides the standard configuration files for
+network filter rules for ensuring clean VM traffic./dd
+
Re: [libvirt] [PATCH] qemu: set default name for SPICE agent channel when generating command
On 03/30/2012 11:06 AM, Michal Privoznik wrote:
On 30.03.2012 09:50, Laine Stump wrote:
commit b0e2bb33 set a default value for the SPICE agent channel by
inserting it during parsing of the channel XML. That method of setting
a default is problematic because it makes a format/parse roundtrip
unclean, and experience with setting other values as a side effect of
parsing has led to headaches (e.g. automatically setting a MAC address
in the parser when one isn't specified in the input XML).
This patch reverts commit b0e2bb33 and replaces it with the alternate
implementation of simply inserting the default value in the
appropriate place on the qemu commandline when no value is provided.
---
(Playing the devil's advocate on my own patch for a moment - one
advantage of Christophe's method of setting the default is that if we
use that object somewhere else in libvirt's code in the future, the
value will be set even if the XML left it unset, but with my method we
will have to check for a NULL name and replace it with the default
value anywhere we want to use it. So I won't say that either method is
definitely the proper way to go, but will just present this
alternative and see if someone else has an even stronger opinion than
me :-)
(BTW, I think I've decided while typing this message that, if we
decide to change from the original method of setting default to this
new method, the change should be pushed as two separate patches - one
reverting the original, and another adding the new. It's too close to
morning for me to take the time to do that right now, though.)
src/conf/domain_conf.c |7 ---
src/qemu/qemu_command.c |6 +++---
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24e10e6..ea558bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5248,13 +5248,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
goto error;
} else {
def-source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT;
-if (!def-target.name) {
-def-target.name = strdup(com.redhat.spice.0);
-if (!def-target.name) {
-virReportOOMError();
-goto error;
-}
-}
}
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..50b1e6d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3457,9 +3457,9 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev,
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC))) {
virBufferAsprintf(buf, ,chardev=char%s,id=%s,
dev-info.alias, dev-info.alias);
-if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
-dev-target.name) {
-virBufferAsprintf(buf, ,name=%s, dev-target.name);
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) {
+virBufferAsprintf(buf, ,name=%s, dev-target.name
+ ? dev-target.name : com.redhat.spice.0);
}
} else {
virBufferAsprintf(buf, ,id=%s, dev-info.alias);
ACK
Maybe it's worth squashing this in:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3d2bb6b..3a14727 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3444,8 +3444,7 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr
dev,
if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL
dev-source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC
-dev-target.name
-STRNEQ(dev-target.name, com.redhat.spice.0)) {
+STRNEQ_NULLABLE(dev-target.name, com.redhat.spice.0)) {
Sure, that's a nice reduction in line count, and optimization.
I pushed it with that change.
Thanks! (to Eric and Christophe as well).
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On 03/30/2012 10:39 AM, Daniel P. Berrange wrote:
On Fri, Mar 30, 2012 at 10:30:01AM -0400, Laine Stump wrote:
From 3b277f2c2138b98145d6b5e18c844be5d43b8b29 Mon Sep 17 00:00:00 2001
From: Laine Stump la...@laine.org
Date: Fri, 30 Mar 2012 10:13:37 -0400
Subject: [PATCH] qemu: add audit logs when switching bridges
This adds in a standard audit log for detaching and attaching a
network device when the bridge being used is changed.
The discussion about this led to the idea that the audit logs being
generated are insufficient, since they don't say anything about which
tap device is used, nor about which bridge it is attached to, but that
should be fixed by a separate patch, and this gets the current patch
properly wired into the infrastructure.
---
src/qemu/qemu_hotplug.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 66837c4..7699e9d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1194,7 +1194,8 @@ static virDomainNetDefPtr
qemuDomainFindNet(virDomainObjPtr vm,
}
static
-int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+int qemuDomainChangeNetBridge(virDomainObjPtr vm,
+ virDomainNetDefPtr olddev,
virDomainNetDefPtr newdev)
{
const char *oldbridge = olddev-data.bridge.brname;
@@ -1221,9 +1222,11 @@ int qemuDomainChangeNetBridge(virDomainNetDefPtr
olddev,
}
return -1;
}
+virDomainAuditNet(vm, NULL, olddev, detach, true);
This needs to be moved up, otherwise if we successfully detach, but
fail to attach we'll not emit the audit message. Also we I think
we should log detach, false if detaching fails, and likewise
'attach', false' if attaching fails
Okay, I've redone the auditing calls, placing them immediately after
each call to add/remove port, calling regardless of whether the
operation was successful or not. I haven't addressed the higher level
problem of adding bridge info to the audit message (although I have made
sure that the NetDef object sent to the audit function has the proper
information - that's why the brname is moved around at different times
than previously).
Does this (squashed into the initial patch that had no auditing) look
adequate?
(I've tested this code, including failure to attach to the new bridge
(by trying to attach to eth0), and it does work as you suggest).
From 7fc635f4234963abad95b66f29bcaa7d02d78de2 Mon Sep 17 00:00:00 2001
From: Laine Stump la...@laine.org
Date: Fri, 30 Mar 2012 10:13:37 -0400
Subject: [PATCH] qemu: add audit logs when switching bridges
This adds in a standard audit log for detaching and attaching a
network device when the bridge being used is changed.
All *attempts* to detach or attach a tap to a bridge are logged, along
with whether or not they are successful.
The discussion about this led to the idea that the audit logs being
generated are insufficient, since they don't say anything about which
tap device is used, nor about which bridge it is attached to, but that
should be fixed by a separate patch, and this gets the current patch
properly wired into the infrastructure.
---
src/qemu/qemu_hotplug.c | 36
1 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ce7921c..80b53d7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1194,11 +1194,13 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm,
}
static
-int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
+int qemuDomainChangeNetBridge(virDomainObjPtr vm,
+ virDomainNetDefPtr olddev,
virDomainNetDefPtr newdev)
{
-const char *oldbridge = olddev-data.bridge.brname;
-const char *newbridge = newdev-data.bridge.brname;
+int ret = -1;
+char *oldbridge = olddev-data.bridge.brname;
+char *newbridge = newdev-data.bridge.brname;
VIR_DEBUG(Change bridge for interface %s: %s - %s,
olddev-ifname, oldbridge, newbridge);
@@ -1209,20 +1211,31 @@ int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
return -1;
}
-if (oldbridge
-virNetDevBridgeRemovePort(oldbridge, olddev-ifname) 0) {
-return -1;
+if (oldbridge) {
+ret = virNetDevBridgeRemovePort(oldbridge, olddev-ifname);
+virDomainAuditNet(vm, olddev, NULL, detach, ret == 0);
+if (ret 0)
+return ret;
}
-if (virNetDevBridgeAddPort(newbridge, olddev-ifname) 0) {
-if (virNetDevBridgeAddPort(oldbridge, olddev-ifname) 0) {
+
+/* move newbridge into olddev now so Audit log is correct */
+olddev-data.bridge.brname = newbridge;
+ret = virNetDevBridgeAddPort(newbridge, olddev-ifname);
+virDomainAuditNet(vm, NULL, olddev, attach, ret == 0);
+if (ret 0)
[libvirt] [PATCH] python: improve conversion validation
Laszlo Ersek pointed out that in trying to convert a long to an
unsigned int, we used:
long long_val = ...;
if ((unsigned int)long_val == long_val)
According to C99 integer promotion rules, the if statement is
equivalent to:
(unsigned long)(unsigned int)long_val == (unsigned long)long_val
since you get an unsigned comparison if at least one side is
unsigned, using the largest rank of the two sides; but on 32-bit
platforms, where unsigned long and unsigned int are the same size,
this comparison is always true and ends up converting negative
long_val into posigive unsigned int values, rather than rejecting
the negative value as we had originally intended (python longs
are unbounded size, and we don't want to do silent modulo
arithmetic when converting to C code).
Fix this by using direct comparisons, rather than casting.
* python/typewrappers.c (libvirt_intUnwrap, libvirt_uintUnwrap)
(libvirt_ulongUnwrap, libvirt_ulonglongUnwrap): Fix conversion
checks.
---
python/typewrappers.c | 27 ---
1 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/python/typewrappers.c b/python/typewrappers.c
index af209e6..4ae3ee1 100644
--- a/python/typewrappers.c
+++ b/python/typewrappers.c
@@ -132,7 +132,7 @@ libvirt_intUnwrap(PyObject *obj, int *val)
if ((long_val == -1) PyErr_Occurred())
return -1;
-if ((int)long_val == long_val) {
+if (long_val = INT_MAX long_val = INT_MIN) {
*val = long_val;
} else {
PyErr_SetString(PyExc_OverflowError,
@@ -151,7 +151,7 @@ libvirt_uintUnwrap(PyObject *obj, unsigned int *val)
if ((long_val == -1) PyErr_Occurred())
return -1;
-if ((unsigned int)long_val == long_val) {
+if (long_val = 0 long_val = UINT_MAX) {
*val = long_val;
} else {
PyErr_SetString(PyExc_OverflowError,
@@ -183,7 +183,13 @@ libvirt_ulongUnwrap(PyObject *obj, unsigned long *val)
if ((long_val == -1) PyErr_Occurred())
return -1;
-*val = long_val;
+if (long_val = 0) {
+*val = long_val;
+} else {
+PyErr_SetString(PyExc_OverflowError,
+negative Python int cannot be converted to C unsigned
long);
+return -1;
+}
return 0;
}
@@ -207,16 +213,23 @@ int
libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val)
{
unsigned long long ullong_val = -1;
+long long llong_val;
/* The PyLong_AsUnsignedLongLong doesn't check the type of
* obj, only accept argument of PyLong_Type, so we check it instead.
*/
-if (PyInt_Check(obj))
-ullong_val = PyInt_AsLong(obj);
-else if (PyLong_Check(obj))
+if (PyInt_Check(obj)) {
+llong_val = PyInt_AsLong(obj);
+if (llong_val 0)
+PyErr_SetString(PyExc_OverflowError,
+negative Python int cannot be converted to C
unsigned long long);
+else
+ullong_val = llong_val;
+} else if (PyLong_Check(obj)) {
ullong_val = PyLong_AsUnsignedLongLong(obj);
-else
+} else {
PyErr_SetString(PyExc_TypeError, an integer is required);
+}
if ((ullong_val == -1) PyErr_Occurred())
return -1;
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On 03/30/2012 12:53 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There are a number of flaws with our packaging of the libvirtd
daemon:
- Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
etc which are required to actually run the hypervisor in
question
- Installing 'libvirt' pulls in the default configuration
files which may not be wanted cause problems if installed
inside a guest
- It is not possible to explicitly required all the peices
required to manage a specific hypervisor
This change takes the 'libvirt' RPM and and changes it thus
- libvirt: just a virtual package with dep on libvirt-daemon,
libvirt-daemon-config-network libvirt-daemon-config-nwfilter
- libvirt-daemon: the libvirt daemon and related pieces
- libvirt-daemon-config-network: the default network config
- libvirt-daemon-config-nwfilter: the network filter configs
- libvirt-docs: the website HTML
We then introduce some more virtual (empty) packages
- libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu'
- libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm'
- libvirt-daemon-lxc: Deps on libvirt-daemon
- libvirt-daemon-uml: Deps on libvirt-daemon
- libvirt-daemon-xen: Deps on libvirt-daemon 'xen'
- libvirt-qemu: Deps on libvirt-daemon-qemu
libvirt-daemon-config-{network,nwfilter}
- libvirt-kvm: Deps on libvirt-daemon-kvm
libvirt-daemon-config-{network,nwfilter}
- libvirt-lxc: Deps on libvirt-daemon-lxc
libvirt-daemon-config-{network,nwfilter}
- libvirt-uml: Deps on libvirt-daemon-uml
libvirt-daemon-config-{network,nwfilter}
- libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-config-network
My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.
It is recommended that applications required a locally installed
libvirtd daemon, use either 'Requires: libvirt-daemon-' or
'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon
or 'Requires: libvirt'
I did a successful make rpm on Fedora 16, which resulted in 21 rpm's
(including the source rpm).
When I tried to update using rpm -U, I was told that I didn't have the
required qemu and xen packages installed (not surprising, since I
never use them). Installing qemu cost 52MB on my disk, and xen another
27. This isn't a bother to me, but I suppose it could be for someone who
was trying to make images as small as possible. On the other hand, that
person will just need to start using the libvirt-kvm package instead
of libvirt.
I guess the only concern I have left is whether or not this change in
package grouping will create any unsavory situations when somebody upgrades.
(there are a few lines in the docs that are still 80 characters (for
some reason, all of them end in binaries)).
Beyond that, it all looks good to me and I'm willing to give my ACK for
what it's worth, but I think it would be a very good idea to have it
looked over by someone with a better knowledge of specfiles than mine.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt PATCHv8 1/1] add DHCP snooping
This patch adds DHCP snooping support to libvirt. The learning method for IP addresses is specified by setting the ip_learning variable to one of any [default] (existing IP learning code), none (static only addresses) or dhcp (DHCP snooping). Active leases are saved in a lease file and reloaded on restart or HUP. Changes since v7: - renamed functions as suggested - collected local state into virNWFilterSnoopState struct - cleaned up include file list - misc code cleanups per review comments Signed-off-by: David L Stevens dlstev...@us.ibm.com --- docs/formatnwfilter.html.in| 17 + po/POTFILES.in |1 + src/Makefile.am|2 + src/nwfilter/nwfilter_dhcpsnoop.c | 1197 src/nwfilter/nwfilter_dhcpsnoop.h | 38 + src/nwfilter/nwfilter_driver.c |6 + src/nwfilter/nwfilter_gentech_driver.c | 59 ++- 7 files changed, 1307 insertions(+), 13 deletions(-) create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 9cb7644..ad10765 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -2189,6 +2189,23 @@ br/br/ In case a VM is resumed after suspension or migrated, IP address detection will be restarted. + br/br/ + Variable iip_learning/i may be used to specify + the IP address learning method. Valid values are iany/i, idhcp/i, + or inone/i. The default value is iany/i, meaning that libvirt + may use any packet to determine the address in use by a VM. A value of + idhcp/i specifies that libvirt should only honor DHCP server-assigned + addresses with valid leases. If iip_learning/i is set to inone/i, + libvirt does not do address learning and referencing iIP/i without + assigning it an explicit value is an error. + br/br/ + Use of iip_learning=dhcp/i (DHCP snooping) provides additional + anti-spoofing security, especially when combined with a filter allowing + only trusted DHCP servers to assign addresses. If the DHCP lease expires, + the VM will no longer be able to use the IP address until it acquires a + new, valid lease from a DHCP server. If the VM is migrated, it must get + a new valid DHCP lease to use an IP address (e.g., by + bringing the VM interface down and up again). /p h3a name=nwflimitsmigrVM Migration/a/h3 diff --git a/po/POTFILES.in b/po/POTFILES.in index 88be04e..d4b0a86 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -51,6 +51,7 @@ src/node_device/node_device_hal.c src/node_device/node_device_linux_sysfs.c src/node_device/node_device_udev.c src/nodeinfo.c +src/nwfilter/nwfilter_dhcpsnoop.c src/nwfilter/nwfilter_driver.c src/nwfilter/nwfilter_ebiptables_driver.c src/nwfilter/nwfilter_gentech_driver.c diff --git a/src/Makefile.am b/src/Makefile.am index a2aae9d..4382caf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -509,6 +509,8 @@ NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ nwfilter/nwfilter_gentech_driver.c \ nwfilter/nwfilter_gentech_driver.h \ + nwfilter/nwfilter_dhcpsnoop.c \ + nwfilter/nwfilter_dhcpsnoop.h \ nwfilter/nwfilter_ebiptables_driver.c \ nwfilter/nwfilter_ebiptables_driver.h \ nwfilter/nwfilter_learnipaddr.c \ diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c new file mode 100644 index 000..8c2ff50 --- /dev/null +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -0,0 +1,1197 @@ +/* + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM + * on an interface + * + * Copyright (C) 2011 IBM Corp. + * Copyright (C) 2011 David L Stevens + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: David L Stevens dlstev...@us.ibm.com + * Based in part on work
Re: [libvirt] [PATCH] Refactor the libvirt RPM daemon pieces
On Fri, Mar 30, 2012 at 03:00:03PM -0400, Laine Stump wrote:
On 03/30/2012 12:53 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There are a number of flaws with our packaging of the libvirtd
daemon:
- Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
etc which are required to actually run the hypervisor in
question
- Installing 'libvirt' pulls in the default configuration
files which may not be wanted cause problems if installed
inside a guest
- It is not possible to explicitly required all the peices
required to manage a specific hypervisor
This change takes the 'libvirt' RPM and and changes it thus
- libvirt: just a virtual package with dep on libvirt-daemon,
libvirt-daemon-config-network libvirt-daemon-config-nwfilter
- libvirt-daemon: the libvirt daemon and related pieces
- libvirt-daemon-config-network: the default network config
- libvirt-daemon-config-nwfilter: the network filter configs
- libvirt-docs: the website HTML
We then introduce some more virtual (empty) packages
- libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu'
- libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm'
- libvirt-daemon-lxc: Deps on libvirt-daemon
- libvirt-daemon-uml: Deps on libvirt-daemon
- libvirt-daemon-xen: Deps on libvirt-daemon 'xen'
- libvirt-qemu: Deps on libvirt-daemon-qemu
libvirt-daemon-config-{network,nwfilter}
- libvirt-kvm: Deps on libvirt-daemon-kvm
libvirt-daemon-config-{network,nwfilter}
- libvirt-lxc: Deps on libvirt-daemon-lxc
libvirt-daemon-config-{network,nwfilter}
- libvirt-uml: Deps on libvirt-daemon-uml
libvirt-daemon-config-{network,nwfilter}
- libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-config-network
My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.
It is recommended that applications required a locally installed
libvirtd daemon, use either 'Requires: libvirt-daemon-' or
'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon
or 'Requires: libvirt'
I did a successful make rpm on Fedora 16, which resulted in 21 rpm's
(including the source rpm).
When I tried to update using rpm -U, I was told that I didn't have the
required qemu and xen packages installed (not surprising, since I
never use them). Installing qemu cost 52MB on my disk, and xen another
27. This isn't a bother to me, but I suppose it could be for someone who
was trying to make images as small as possible. On the other hand, that
person will just need to start using the libvirt-kvm package instead
of libvirt.
I assume that is because you did 'rpm -Uvh *.rpm', which means it
would have included libvirt-qemu and libvirt-xen which in turn
dep on 'qemu' and 'xen' respectively. If you had merely asked to
install 'libvirt', it would not have required libvirt-qemu or
libvirt-xen.
To clarify the dependancy chain is this:
libvirt - libvirt-daemon, libvirt-config-network, libvirt-config-nwfilter
libvirt-daemon-qemu - libvirt-daemon, qemu
libvirt-daemon-kvm - libvirt-daemon, qemu-kvm
libvirt-daemon-xen - libvirt-daemon, xen
libvirt-qemu - libvirt-daemon-qemu, libvirt-config-network,
libvirt-config-nwfilter
libvirt-kvm - libvirt-daemon-kvm, libvirt-config-network,
libvirt-config-nwfilter
libvirt-xen - libvirt-daemon-xen, libvirt-config-network
Notice there is no direct dep from 'libvirt' to 'qemu' or 'xen', only
from the libvirt-qemu RPMs, which are not required by anything else.
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] Refactor the libvirt RPM daemon pieces
On 03/30/2012 03:30 PM, Daniel P. Berrange wrote:
On Fri, Mar 30, 2012 at 03:00:03PM -0400, Laine Stump wrote:
On 03/30/2012 12:53 PM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There are a number of flaws with our packaging of the libvirtd
daemon:
- Installing 'libvirt' does not install 'qemu-kvm' or 'xen'
etc which are required to actually run the hypervisor in
question
- Installing 'libvirt' pulls in the default configuration
files which may not be wanted cause problems if installed
inside a guest
- It is not possible to explicitly required all the peices
required to manage a specific hypervisor
This change takes the 'libvirt' RPM and and changes it thus
- libvirt: just a virtual package with dep on libvirt-daemon,
libvirt-daemon-config-network libvirt-daemon-config-nwfilter
- libvirt-daemon: the libvirt daemon and related pieces
- libvirt-daemon-config-network: the default network config
- libvirt-daemon-config-nwfilter: the network filter configs
- libvirt-docs: the website HTML
We then introduce some more virtual (empty) packages
- libvirt-daemon-qemu: Deps on libvirt-daemon 'qemu'
- libvirt-daemon-kvm: Deps on libvirt-daemon 'qemu-kvm'
- libvirt-daemon-lxc: Deps on libvirt-daemon
- libvirt-daemon-uml: Deps on libvirt-daemon
- libvirt-daemon-xen: Deps on libvirt-daemon 'xen'
- libvirt-qemu: Deps on libvirt-daemon-qemu
libvirt-daemon-config-{network,nwfilter}
- libvirt-kvm: Deps on libvirt-daemon-kvm
libvirt-daemon-config-{network,nwfilter}
- libvirt-lxc: Deps on libvirt-daemon-lxc
libvirt-daemon-config-{network,nwfilter}
- libvirt-uml: Deps on libvirt-daemon-uml
libvirt-daemon-config-{network,nwfilter}
- libvirt-xen: Deps on libvirt-daemon-xen libvirt-daemon-config-network
My intent in the future is to turn on the driver modules by
default, at which time 'libvirt-daemon' will cease to include
any specific drivers, instead we'll get libvirt-daemon-driver-
packages for each driver. The libvirt-daemon-XXX packages will
then pull in each driver that they require.
It is recommended that applications required a locally installed
libvirtd daemon, use either 'Requires: libvirt-daemon-' or
'Requires: libvirt-XXX' and *not* Requires: libvirt-daemon
or 'Requires: libvirt'
I did a successful make rpm on Fedora 16, which resulted in 21 rpm's
(including the source rpm).
When I tried to update using rpm -U, I was told that I didn't have the
required qemu and xen packages installed (not surprising, since I
never use them). Installing qemu cost 52MB on my disk, and xen another
27. This isn't a bother to me, but I suppose it could be for someone who
was trying to make images as small as possible. On the other hand, that
person will just need to start using the libvirt-kvm package instead
of libvirt.
I assume that is because you did 'rpm -Uvh *.rpm', which means it
would have included libvirt-qemu and libvirt-xen which in turn
dep on 'qemu' and 'xen' respectively. If you had merely asked to
install 'libvirt', it would not have required libvirt-qemu or
libvirt-xen.
You assume correctly. I had always (incorrectly) assumed that rpm -U
would only update the machine rpms that were already installed. I'd
never actually checked.
So everything with this change seems okay to me.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix deadlock when qemu cannot start
Quoting Daniel Veillard (veill...@redhat.com): On Fri, Mar 30, 2012 at 08:45:35AM -0500, Serge Hallyn wrote: Quoting Daniel Veillard (veill...@redhat.com): On Fri, Mar 16, 2012 at 02:37:42PM +0800, Wen Congyang wrote: When qemu cannot start, we may call qemuProcessStop() twice. We have check whether the vm is running at the beginning of qemuProcessStop() to avoid libvirt deadlock. We call qemuProcessStop() with driver and vm locked. It seems that we can avoid libvirt deadlock. But unfortunately we may unlock driver and vm in the function qemuProcessKill() while vm-def-id is not -1. So qemuProcessStop() will be run twice, and monitor will be freed unexpectedly. So we should set vm-def-id to -1 at the beginning of qemuProcessStop(). Oh, uh, Huh. This seems like it could be responsible for what I was reporting a few days ago (*1). I spent most of yesterday trying to track it down, only to finally realize that the QemuProcessStart would silently die at various places. So i was getting ready to send an email postulating that what's happening is that a virsh list starts, then a virsh start starts, and when the virsh list ends it somehow causes the virsh start to be killed. I'll test and see if this patch fixes it. I guess I need to resurect my patch checker, tune it and make it send warning on patches not ACK'ed or reviewed automatically, or even better make it a web page to avoid boring people. http://libvirt.org/git/?p=patchchecker.git;a=summary I had that patch on my radar and though I took an awful long time to review it it's there. BTW I'm late for rc2, I will make it tomorrow I guess, sorry about it this is due to events out of my control :-\ Daniel Alas, this patch (by itself) doesn't fix the issue for me. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix mingw ssize_t, syntax check
On 03/30/2012 10:54 AM, Daniel P. Berrange wrote: On Fri, Mar 30, 2012 at 10:35:41AM -0600, Eric Blake wrote: We are so close to a release that we don't want to pull in a gnulib submodule update and risk regressions, since there has been a lot of other gnulib churn upstream. However, there are a couple of gnulib issues that are worth fixing in isolation, by applying local patches to gnulib. +$(MAKE) $(release-prep-hook) RELEASE_TYPE=$@ +$(MAKE) -s emit_upload_commands RELEASE_TYPE=$@ + diff --git a/src/libvirt.c b/src/libvirt.c This diff added a trailing blank line, as well as some space-tab indentation. ACK I made an additional tweak to cfg.mk to avoid syntax-check failures, as well as exempting gnulib/local/*/*.diff from whitespace checking, then pushed. I suspect we will revert the local diffs when we next update the .gnulib submodule, but that will be post-0.9.11. -- Eric Blake ebl...@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] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On 03/30/2012 12:22 PM, Laine Stump wrote: Subject: [PATCH] qemu: add audit logs when switching bridges This adds in a standard audit log for detaching and attaching a network device when the bridge being used is changed. All *attempts* to detach or attach a tap to a bridge are logged, along with whether or not they are successful. The discussion about this led to the idea that the audit logs being generated are insufficient, since they don't say anything about which tap device is used, nor about which bridge it is attached to, but that should be fixed by a separate patch, and this gets the current patch properly wired into the infrastructure. --- src/qemu/qemu_hotplug.c | 36 1 files changed, 24 insertions(+), 12 deletions(-) ACK. This looks like you have correctly called into the audit framework, and I agree that a later patch to improve the audit framework to output additional useful information is now possible. I recommend that get this in before rc2. -- Eric Blake ebl...@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] Refactor the libvirt RPM daemon pieces
On 03/30/2012 10:53 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
There are a number of flaws with our packaging of the libvirtd
daemon:
+
+%if %{with_libvirtd}
+%package daemon
+Summary: Server side daemon and supporting files for libvirt library
+Group: Development/Libraries
+
+# All runtime requirements for the libvirt package (runtime requrements
+# for subpackages are listed later in those subpackages)
+
+# The client side, i.e. shared libs and virsh are in a subpackage
+Requires: %{name}-client = %{version}-%{release}
+
+# Used by many of the drivers, so turn it on whenever the
+# daemon is present
+%if %{with_libvirtd}
You've now nested %if %{with_libvirtd} twice; this could be simplified,
but cleanups can be saved for another day if we are under the gun to get
this out now (is there a cppi equivalent for spec files to help show
nesting? Can you even indent %if directives for sanity in a spec file?)
+# for modprobe of pci devices
...
+# For glusterfs
+%if 0%{?fedora} = 11
+Requires: glusterfs-client = 2.0.1
+%endif
+%endif
+%if %{with_qemu}
+# From QEMU RPMs
+Requires: /usr/bin/qemu-img
Should this Requires: be pushed down to libvirt-daemon-qemu...
+# For image compression
+Requires: gzip
+Requires: bzip2
+Requires: lzop
+Requires: xz
+%else
+%if %{with_xen}
+# From Xen RPMs
+Requires: /usr/sbin/qcow-create
and this one to libvirt-daemon-kvm?
But I can live with them here.
...
+# For virConnectGetSysinfo
+Requires: dmidecode
+%endif
+# For service management
+%if %{with_systemd}
+Requires(post): systemd-units
+Requires(post): systemd-sysv
+Requires(preun): systemd-units
+Requires(postun): systemd-units
These four requires deal with services, but we already pushed services
into libvirt-daemon-config-network; I'm guessing these should be
relocated to that section.
Or maybe I'm misunderstanding - is it that these enable libvirtd to run
as a service, but libvirtd will do nothing to cause interference (like
trying to set up the 'default' NAT network at 192.168.122.0) unless you
also have the config files? In that case, what you did looks okay.
+%endif
+%if %{with_numad}
+Requires: numad
+%endif
+
+%description daemon
+Server side daemon required to manage the virtualization capabilities
+of recent versions of Linux. Requires a hypervisor specific sub-RPM
+for specific drivers.
It looks weird to have the %description so far down from the %package
with all the Requires in between. But that's cosmetic.
I spotted a few things you can clean up, but nothing was a show-stopper
to pushing this as-is, so you have my ACK.
--
Eric Blake ebl...@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] build: fix build on cygwin
Regression introduced when we changed types in commit 3e2c3d8f6.
We've done this sort of cleanup before (see commit c685993d7).
* src/conf/storage_conf.c (virStoragePoolDefFormat)
(virStorageVolTargetDefFormat): Cast gid_t and uid_t.
---
Pushing under the build-breaker rule.
src/conf/storage_conf.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index bdf6218..2330fa1 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -915,10 +915,10 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) {
virBufferAddLit(buf,permissions\n);
virBufferAsprintf(buf, mode0%o/mode\n,
def-target.perms.mode);
-virBufferAsprintf(buf, owner%d/owner\n,
- def-target.perms.uid);
-virBufferAsprintf(buf, group%d/group\n,
- def-target.perms.gid);
+virBufferAsprintf(buf, owner%u/owner\n,
+ (unsigned int) def-target.perms.uid);
+virBufferAsprintf(buf, group%u/group\n,
+ (unsigned int) def-target.perms.gid);
if (def-target.perms.label)
virBufferAsprintf(buf, label%s/label\n,
@@ -1152,10 +1152,10 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr
options,
virBufferAddLit(buf,permissions\n);
virBufferAsprintf(buf, mode0%o/mode\n,
def-perms.mode);
-virBufferAsprintf(buf, owner%d/owner\n,
- def-perms.uid);
-virBufferAsprintf(buf, group%d/group\n,
- def-perms.gid);
+virBufferAsprintf(buf, owner%u/owner\n,
+ (unsigned int) def-perms.uid);
+virBufferAsprintf(buf, group%u/group\n,
+ (unsigned int) def-perms.gid);
if (def-perms.label)
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] openvz: Allow missing micro tag in the version
No reason to cause the openvz driver to fail on startup if the micro tag in missed. This solves the problem: https://www.redhat.com/archives/libvirt-users/2012-March/msg00202.html --- src/openvz/openvz_conf.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 28f86ff..da6869b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -95,11 +95,13 @@ openvzExtractVersionInfo(const char *cmdstr, int *retversion) tmp = help; -/* expected format: vzctl version major.minor.micro */ +/* expected format: vzctl version major.minor.micro, + * missing micro is allowed. + */ if ((tmp = STRSKIP(tmp, vzctl version )) == NULL) goto cleanup; -if (virParseVersionString(tmp, version, false) 0) +if (virParseVersionString(tmp, version, true) 0) goto cleanup; if (retversion) -- 1.7.7.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device
On 03/30/2012 06:23 PM, Eric Blake wrote: On 03/30/2012 12:22 PM, Laine Stump wrote: Subject: [PATCH] qemu: add audit logs when switching bridges This adds in a standard audit log for detaching and attaching a network device when the bridge being used is changed. All *attempts* to detach or attach a tap to a bridge are logged, along with whether or not they are successful. The discussion about this led to the idea that the audit logs being generated are insufficient, since they don't say anything about which tap device is used, nor about which bridge it is attached to, but that should be fixed by a separate patch, and this gets the current patch properly wired into the infrastructure. --- src/qemu/qemu_hotplug.c | 36 1 files changed, 24 insertions(+), 12 deletions(-) ACK. This looks like you have correctly called into the audit framework, and I agree that a later patch to improve the audit framework to output additional useful information is now possible. I recommend that get this in before rc2. Thanks to Hendrik Schwartke for writing the original patch, and to Eric and Dan for the reviews and suggestions! I squashed the Auditing code into my earlier modification of Hendrik's bridge-change patch, and pushed (I also added references to the two open BZes that are related to this functionality, one upstream and one for RHEL). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: change rbd auth_supported separation character to ;
This works with newer qemu that doesn't allow escaping spaces.
It's backwards compatible as well.
Signed-off-by: Josh Durgin josh.dur...@dreamhost.com
---
src/qemu/qemu_command.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f971a08..ee3bf48 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1673,8 +1673,8 @@ qemuBuildRBDString(virConnectPtr conn,
virReportOOMError();
goto error;
}
-virBufferEscape(opt, '\\', :,
-:key=%s:auth_supported=cephx none,
+virBufferEscape(opt, '\\', :;,
+:key=%s:auth_supported=cephx;none,
base64);
VIR_FREE(base64);
} else {
--
1.7.5.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: allow snapshotting of sheepdog and rbd disks
Signed-off-by: Josh Durgin josh.dur...@dreamhost.com
---
.gnulib|2 +-
src/qemu/qemu_driver.c | 14 ++
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/.gnulib b/.gnulib
index d5612c7..6b93d00 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit d5612c714c87555f1059d71d347e20271dced322
+Subproject commit 6b93d00f5410ec183e3a70ebf8e418e3b1bb0191
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7e6d59c..fc537df 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9571,12 +9571,18 @@ qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
* that succeed as well
*/
for (i = 0; i vm-def-ndisks; i++) {
-if ((vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_LUN) ||
-(vm-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_DISK
- STRNEQ_NULLABLE(vm-def-disks[i]-driverType, qcow2))) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK
+(disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG ||
+ disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD))
+continue;
+
+if ((disk-device == VIR_DOMAIN_DISK_DEVICE_LUN) ||
+(disk-device == VIR_DOMAIN_DISK_DEVICE_DISK
+ STRNEQ_NULLABLE(disk-driverType, qcow2))) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
_(Disk '%s' does not support snapshotting),
-vm-def-disks[i]-src);
+disk-src);
return false;
}
}
--
1.7.5.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] conf: allow fuzz in XML with cur balloon max
Thanks for your patch serials.
I think they fix the true bug.
But I have a little doubt on the fuzz allowance, pls have a see
comment in line below.
On Fri, Mar 30, 2012 at 11:56 PM, Eric Blake ebl...@redhat.com wrote:
Commit 1b1402b introduced a regression. Since older libvirt versions
would silently round memory up (until the previous patch), but populated
current memory based on querying the guest, it was possible to have
current maximum by the amount of the rounding. Accept this fuzz
factor, and silently clamp current down to maximum in that case,
rather than failing to reparse XML for an existing VM.
* src/conf/domain_conf.c (virDomainDefParseXML): Don't reject
existing XML.
Based on a report by Zhou Peng.
---
src/conf/domain_conf.c | 19 +++
src/qemu/qemu_monitor_json.c | 2 +-
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4caef6f..7c95920 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7776,10 +7776,21 @@ static virDomainDefPtr
virDomainDefParseXML(virCapsPtr caps,
goto error;
if (def-mem.cur_balloon def-mem.max_balloon) {
- virDomainReportError(VIR_ERR_XML_ERROR,
- _(current memory '%lluk' exceeds maximum
'%lluk'),
- def-mem.cur_balloon, def-mem.max_balloon);
- goto error;
+ /* Older libvirt could get into this situation due to
Do you mean to allow the scene:
* The checkpoint( thought invalid by old libvirt) produced by older libvirt
can be thought valid if restored by the patched newer libvirt?
But it will also allow a typo xml.
* Another scene may be live migration between old libvirt
and newer libvirt (pls correct me if err)
If so, I think these scene should be documented or commented here or
in the commit msg explicitly, otherwise this piece of code may confuse many
readers, because I don't think cur_balloon can exceed max_balloon
again after patched
(so if exceed must be typo xml).
+ * rounding; if the discrepancy is less than 1MiB, we silently
+ * round down, otherwise we flag the issue. */
+ if (VIR_DIV_UP(def-mem.cur_balloon, 1024)
+ VIR_DIV_UP(def-mem.max_balloon, 1024)) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _(current memory '%lluk' exceeds
+ maximum '%lluk'),
+ def-mem.cur_balloon, def-mem.max_balloon);
+ goto error;
+ } else {
+ VIR_DEBUG(Truncating current %lluk to maximum %lluk,
+ def-mem.cur_balloon, def-mem.max_balloon);
+ def-mem.cur_balloon = def-mem.max_balloon;
+ }
} else if (def-mem.cur_balloon == 0) {
def-mem.cur_balloon = def-mem.max_balloon;
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7093e1d..6d66587 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2022,7 +2022,7 @@ int qemuMonitorJSONSetBalloon(qemuMonitorPtr mon,
{
int ret;
virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(balloon,
- U:value, ((unsigned
long long)newmem)*1024,
+ U:value,
newmem*1024ULL,
NULL);
virJSONValuePtr reply = NULL;
if (!cmd)
--
1.7.1
--
Zhou Peng
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
