Re: [libvirt] [PATCH] build: fix parted detection at configure time
On 01/31/2011 05:10 PM, Eric Blake wrote: * configure.ac (PARTED_FOUND): Issue configure error if --with-storage-disk=yes but no parted is found. --- As pointed out while reviewing Osier's patch, we were unconditionally nuking with_storage_disk=no if parted was not found, then trying to issue AC_MSG_ERROR if the user had passed in yes. Also break some long lines. configure.ac | 15 +-- 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index f310a5e..2873fff 100644 --- a/configure.ac +++ b/configure.ac @@ -1703,17 +1703,18 @@ AC_SUBST([DEVMAPPER_LIBS]) LIBPARTED_CFLAGS= LIBPARTED_LIBS= -if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; then +if test "$with_storage_disk" = "yes" || + test "$with_storage_disk" = "check"; then AC_PATH_PROG([PARTED], [parted], [], [$PATH:/sbin:/usr/sbin]) if test -z "$PARTED" ; then -with_storage_disk=no PARTED_FOUND=no else PARTED_FOUND=yes fi - if test "$with_storage_disk" != "no"&& test "x$PKG_CONFIG" != "x" ; then -PKG_CHECK_MODULES(LIBPARTED, libparted>= $PARTED_REQUIRED, [], [PARTED_FOUND=no]) + if test "$PARTED_FOUND" = yes&& test "x$PKG_CONFIG" != "x" ; then Just one small nit - almost everywhere else in configure.ac, the yes or no is quoted, but here you haven't quoted it. Operationally it makes no difference, it just looks "different" :-) At any rate, ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/5] smartcard: add XML support for device
On Mon, Jan 31, 2011 at 04:40:06PM -0700, Eric Blake wrote: > On 01/26/2011 11:29 AM, Alon Levy wrote: > > yes, the db is a directory name, treated as normal (can be absolute or > > relative > > to cwd, I don't check, just feed it to NSS). It defaults to /etc/pki/nssdb: > > Hmm; given Osier's recent commit cc4447b to allow $HOME/.pki/libvirt to > be tried first for non-root users, should the same logic be employed > here (if nothing is explicitly requested, then favor $HOME/.pki/nssdb > for non-root before falling back to /etc/pki/nssdb)? > Sounds logical I guess. I should update the device to do the same on default (but of course if libvirt supplies a directory it would override the default). > -- > Eric Blake ebl...@redhat.com+1-801-349-2682 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/5] smartcard: add XML support for device
On Mon, Jan 31, 2011 at 04:33:46PM -0700, Eric Blake wrote: > On 01/26/2011 11:29 AM, Alon Levy wrote: > > yes, the db is a directory name, treated as normal (can be absolute or > > relative > > to cwd, I don't check, just feed it to NSS). > > From qemu's point of view, it can be relative; but how does a libvirt > user know what directory libvirt will be running in? Hence in the xml > we might as well enforce that it be absolute, with no loss of > functionality (and gui wrappers around libvirt can use typical file > browser windows to allow relative browsing to locate such a directory). > > > It defaults to /etc/pki/nssdb: > > (certutil needs an argument, we have it #defined: > > hw/ccid-card-emulated.c:#define CERTIFICATES_DEFAULT_DB "/etc/pki/nssdb" > > Okay, I'll add that same default to libvirt. > > >> Should we also have 'database' for the 'host' mode if we need one ? > > Yes, without it the usage of certificates is limited to the default > > certificate > > store, and if anyone wants to run multiple qemu's with different > > certificates they > > may want to put them into different dbs. > > Does qemu accept -device ccid-card-emulated,backend=nss-emulated,db=xyz? > No, the db is only for backend=certificates, I thought that's what we were talking about. > That is, if NSS is using a host USB device, then I don't see what the > use is of providing a database directory in that case. It isn't, see above. > > I don't see a need to add a subelement to mode='host' in the > XML right now; we can leave that as a future enhancement to the XML > without affecting this patch. I'm more worried that this patch does > _not_ include anything that doesn't make sense, than I am about adding > more later if we find we missed something. As long as you are talking about host mode not needing db I'm with you. But certificates mode (i.e. -device ccid-card-emulated,backend=certificates) does. > > -- > Eric Blake ebl...@redhat.com+1-801-349-2682 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4 v2] cgroup: Implement blkio.weight tuning API.
On Thu, 27 Jan 2011 13:18:44 +0800, Gui Jianfeng wrote: > Implement blkio.weight tuning API. > > Signed-off-by: Gui Jianfeng > --- > src/libvirt_private.syms |2 ++ > src/util/cgroup.c| 39 +++ > src/util/cgroup.h|3 +++ > 3 files changed, 44 insertions(+), 0 deletions(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 2ce4bed..97b9851 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -77,6 +77,8 @@ virCgroupMounted; > virCgroupRemove; > virCgroupSetCpuShares; > virCgroupSetFreezerState; > +virCgroupSetWeight; > +virCgroupGetWeight; As and after thought, shouldn't we call this as virCgroupSetBlkioWeight virCgroupGetBlkioWeight The current API does not indicate that it is Block IO weight. Nikunj -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4] bridge_driver: handle DNS over IPv6
On 01/31/2011 03:31 PM, Paweł Krześniak wrote: * dnsmasq listens on all defined IPv[46] addresses for network * Add ip6tables rules to allow DNS traffic to host --- src/network/bridge_driver.c | 51 ++ 1 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c098ab5..24be0b7 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -432,6 +432,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; char *bridgeaddr; +int ii; +virNetworkIpDefPtr tmpipdef; if (!(bridgeaddr = virSocketFormatAddr(&ipdef->address))) goto cleanup; @@ -468,20 +470,28 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, /* *no* conf file */ virCommandAddArgList(cmd, "--conf-file=", "", NULL); -/* - * XXX does not actually work, due to some kind of - * race condition setting up ipv6 addresses on the - * interface. A sleep(10) makes it work, but that's - * clearly not practical - * - * virCommandAddArg(cmd, "--interface"); - * virCommandAddArg(cmd, ipdef->bridge); - */ virCommandAddArgList(cmd, - "--listen-address", bridgeaddr, "--except-interface", "lo", NULL); +/* + * --interface does not actually work with dnsmasq< 2.47, + * due to DAD for ipv6 addresses on the interface. + * + * virCommandAddArgList(cmd, "--interface", ipdef->bridge, NULL); + * + * So listen on all defined IPv[46] addresses + */ +for (ii = 0; + (tmpipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); + ii++) { + char *ipaddr = virSocketFormatAddr(&tmpipdef->address); +if (!ipaddr) +goto cleanup; +virCommandAddArgList(cmd, "--listen-address", ipaddr, NULL); +VIR_FREE(ipaddr); +} + for (r = 0 ; r< ipdef->nranges ; r++) { char *saddr = virSocketFormatAddr(&ipdef->ranges[r].start); if (!saddr) @@ -1027,9 +1037,30 @@ networkAddGeneralIp6tablesRules(struct network_driver *driver, goto err3; } +/* allow DNS over IPv6 */ +if (iptablesAddTcpInput(driver->iptables, AF_INET6, +network->def->bridge, 53)< 0) { +networkReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to add ip6tables rule to allow DNS requests from '%s'"), + network->def->bridge); +goto err4; +} + +if (iptablesAddUdpInput(driver->iptables, AF_INET6, +network->def->bridge, 53)< 0) { +networkReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to add ip6tables rule to allow DNS requests from '%s'"), + network->def->bridge); +goto err5; +} + return 0; /* unwind in reverse order from the point of failure */ +err5: +iptablesRemoveTcpInput(driver->iptables, AF_INET6, network->def->bridge, 53); +err4: +iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6, network->def->bridge); err3: iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6, network->def->bridge); err2: This looks good, builds, passes make syntax-check (after I squashed out one errant tab character), and appears to function properly. ACK, and pushed. Thanks! As I mentioned in earlier mails, sometime in the future a good followup to this for someone would be to eliminate the "ipdef" arg to networkBuildDnsmasqArgv, and figure out how to make dnsmasq serve multiple dhcp segments on the same interface. But that's a separate issue for another day... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Get cpuMhz of virNodeGetInfo() from cpufreq/cpuinfo_max_freq, if exist
Hi, Eric On Mon, 31 Jan 2011 15:46:39 -0700 Eric Blake wrote: > On 01/27/2011 02:51 AM, Minoru Usui wrote: > > virNodeGetInfo() gets from > > /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq, first. > > > > Signed-off-by: Minoru Usui > > I haven't looked closely at this series yet... > > > +/* > > + * nodeinfo->mhz should return maximum frequency, > > + * but "cpu MHz" of /proc/cpuinfo is scaled by power saving feature. > > + * So it gets cpufreq/cpuinfo_max_freq, if possible. > > + */ > > +ret = get_cpu_value(0, "cpufreq/cpuinfo_max_freq", true); > > +if (ret < 0) > > + return -1; > > +else if (ret != 1) { > > + /* convert unit */ > > + cpu_mhz = ret / 1000; > > But which units is this converting between, and should it truncate or > round up? I think it divide by 1000 is collect, because my machine returns below values. - # cat /sys/devices/system/cpu/cpu?/cpufreq/cpuinfo_max_freq 231 231 231 231 231 231 231 231 # grep 'cpu MHz' /proc/cpuinfo cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 cpu MHz : 2333.331 - On the other hand, I don't have clear opinion about truncate or round up. Present implementation of linuxNodeInfoCPUPopulate() selects truncate, so I implement to truncate logic. -- Minoru Usui -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt 0.8.7 LOCALSTATEDIR ubuntu patch
On 01/31/2011 04:57 PM, Philipp Schmid wrote: > -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs > -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings > -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls > -Wno-sign-compare -Wlogical-op -Wp,-D_FORTIFY_SOURCE=2 -fexceptions > -fasynchronous-unwind-tables -fdiagnostics-show-option -DIN_LIBVIRT > -I../src/conf -g -O2 -g -O2 -c nwfilter/nwfilter_ebiptables_driver.c -fPIC > -DPIC -o .libs/libvirt_driver_nwfilter_la-nwfilter_ebiptables_driver.o > nwfilter/nwfilter_ebiptables_driver.c: In function > 'ebiptablesWriteToTempFile': > nwfilter/nwfilter_ebiptables_driver.c:2444:23: error: 'LOCALSTATEDIR' > undeclared (first use in this function) Add this line to the problematic file: #include "configmake.h" (see nwfilter/nwfilter_driver.c for an example). > Author: Jamie Strandboge > Description: move ebiptables script from /tmp to /var/lib/libvirt > Forwarded: yes > > Index: libvirt-0.8.7/src/nwfilter/nwfilter_ebiptables_driver.c > === > --- libvirt-0.8.7.orig/src/nwfilter/nwfilter_ebiptables_driver.c > 2011-01-30 20:57:49.132301999 + > +++ libvirt-0.8.7/src/nwfilter/nwfilter_ebiptables_driver.c 2011-01-30 > 20:58:35.762302000 + > @@ -2441,7 +2441,7 @@ > */ > static char * > ebiptablesWriteToTempFile(const char *string) { > -char filename[] = "/tmp/virtdXX"; > +char filename[] = LOCAL_STATE_DIR "/lib/libvirt/virtdXX"; Better yet, we should probably have some utility function that makes it easy to create a temporary file, while honoring $TMPDIR (if indeed it belongs in $TMPDIR rather than /var/lib), instead of just hard-coding /tmp. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] libvirt 0.8.7 LOCALSTATEDIR ubuntu patch
Hi, I'm using ubuntu for our libvirt/kvm host server and needed IPv6 connectivity, so I started to update the latest ubuntu libvirt package (0.8.5) to 0.8.7. I got all but one ubuntu specific patch to apply. The patch moves the ebiptables script from /tmp to "LOCALSTATEDIR" /lib/libvirt. I renamed LOCAL_STATE_DIR in the patch below to LOCALSTATEDIR, because this seems to have changed between 0.8.5 and 0.8.7, but I still get the following compile error. Since this is my first contact with the libvirt source or build system, I would be happy for any pointers on how to fix this and make LOCALSTATEDIR visible to nwfilter/nwfilter_ebiptables_driver.c. Best regards, Philipp Schmid PS: if anyone is interested in working (at least for me with kvm) packages (without the mentioned patch), you can grab them here: http://www.schmidp.com/public/libvirt_0.8.7_ubuntu_natty/ -Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare -Wlogical-op -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fasynchronous-unwind-tables -fdiagnostics-show-option -DIN_LIBVIRT -I../src/conf -g -O2 -g -O2 -c nwfilter/nwfilter_ebiptables_driver.c -fPIC -DPIC -o .libs/libvirt_driver_nwfilter_la-nwfilter_ebiptables_driver.o nwfilter/nwfilter_ebiptables_driver.c: In function 'ebiptablesWriteToTempFile': nwfilter/nwfilter_ebiptables_driver.c:2444:23: error: 'LOCALSTATEDIR' undeclared (first use in this function) nwfilter/nwfilter_ebiptables_driver.c:2444:23: note: each undeclared identifier is reported only once for each function it appears in nwfilter/nwfilter_ebiptables_driver.c:2444:37: error: expected ',' or ';' before string constant make[4]: *** [libvirt_driver_nwfilter_la-nwfilter_ebiptables_driver.lo] Error 1 make[4]: Leaving directory `/tmp/buildd/libvirt-0.8.7/src' make[3]: *** [all] Error 2 make[3]: Leaving directory `/tmp/buildd/libvirt-0.8.7/src' make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory `/tmp/buildd/libvirt-0.8.7' make[1]: *** [all] Error 2 make[1]: Leaving directory `/tmp/buildd/libvirt-0.8.7' make: *** [debian/stamp-makefile-build] Error 2 # Author: Jamie Strandboge Description: move ebiptables script from /tmp to /var/lib/libvirt Forwarded: yes Index: libvirt-0.8.7/src/nwfilter/nwfilter_ebiptables_driver.c === --- libvirt-0.8.7.orig/src/nwfilter/nwfilter_ebiptables_driver.c 2011-01-30 20:57:49.132301999 + +++ libvirt-0.8.7/src/nwfilter/nwfilter_ebiptables_driver.c 2011-01-30 20:58:35.762302000 + @@ -2441,7 +2441,7 @@ */ static char * ebiptablesWriteToTempFile(const char *string) { -char filename[] = "/tmp/virtdXX"; +char filename[] = LOCAL_STATE_DIR "/lib/libvirt/virtdXX"; int len; char *filnam; virBuffer buf = VIR_BUFFER_INITIALIZER; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/5] smartcard: add XML support for device
On 01/26/2011 11:29 AM, Alon Levy wrote: > yes, the db is a directory name, treated as normal (can be absolute or > relative > to cwd, I don't check, just feed it to NSS). It defaults to /etc/pki/nssdb: Hmm; given Osier's recent commit cc4447b to allow $HOME/.pki/libvirt to be tried first for non-root users, should the same logic be employed here (if nothing is explicitly requested, then favor $HOME/.pki/nssdb for non-root before falling back to /etc/pki/nssdb)? -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [PATCHv3 1/5] smartcard: add XML support for device
On 01/26/2011 11:29 AM, Alon Levy wrote: > yes, the db is a directory name, treated as normal (can be absolute or > relative > to cwd, I don't check, just feed it to NSS). From qemu's point of view, it can be relative; but how does a libvirt user know what directory libvirt will be running in? Hence in the xml we might as well enforce that it be absolute, with no loss of functionality (and gui wrappers around libvirt can use typical file browser windows to allow relative browsing to locate such a directory). > It defaults to /etc/pki/nssdb: > (certutil needs an argument, we have it #defined: > hw/ccid-card-emulated.c:#define CERTIFICATES_DEFAULT_DB "/etc/pki/nssdb" Okay, I'll add that same default to libvirt. >> Should we also have 'database' for the 'host' mode if we need one ? > Yes, without it the usage of certificates is limited to the default > certificate > store, and if anyone wants to run multiple qemu's with different certificates > they > may want to put them into different dbs. Does qemu accept -device ccid-card-emulated,backend=nss-emulated,db=xyz? That is, if NSS is using a host USB device, then I don't see what the use is of providing a database directory in that case. I don't see a need to add a subelement to mode='host' in the XML right now; we can leave that as a future enhancement to the XML without affecting this patch. I'm more worried that this patch does _not_ include anything that doesn't make sense, than I am about adding more later if we find we missed something. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Get cpuMhz of virNodeGetInfo() from cpufreq/cpuinfo_max_freq, if exist
On 01/27/2011 02:51 AM, Minoru Usui wrote: > virNodeGetInfo() gets from > /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq, first. > > Signed-off-by: Minoru Usui I haven't looked closely at this series yet... > +/* > + * nodeinfo->mhz should return maximum frequency, > + * but "cpu MHz" of /proc/cpuinfo is scaled by power saving feature. > + * So it gets cpufreq/cpuinfo_max_freq, if possible. > + */ > +ret = get_cpu_value(0, "cpufreq/cpuinfo_max_freq", true); > +if (ret < 0) > + return -1; > +else if (ret != 1) { > + /* convert unit */ > + cpu_mhz = ret / 1000; But which units is this converting between, and should it truncate or round up? -- Eric Blake ebl...@redhat.com+1-801-349-2682 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 parted detection at configure time
* configure.ac (PARTED_FOUND): Issue configure error if --with-storage-disk=yes but no parted is found. --- As pointed out while reviewing Osier's patch, we were unconditionally nuking with_storage_disk=no if parted was not found, then trying to issue AC_MSG_ERROR if the user had passed in yes. Also break some long lines. configure.ac | 15 +-- 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index f310a5e..2873fff 100644 --- a/configure.ac +++ b/configure.ac @@ -1703,17 +1703,18 @@ AC_SUBST([DEVMAPPER_LIBS]) LIBPARTED_CFLAGS= LIBPARTED_LIBS= -if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; then +if test "$with_storage_disk" = "yes" || + test "$with_storage_disk" = "check"; then AC_PATH_PROG([PARTED], [parted], [], [$PATH:/sbin:/usr/sbin]) if test -z "$PARTED" ; then -with_storage_disk=no PARTED_FOUND=no else PARTED_FOUND=yes fi - if test "$with_storage_disk" != "no" && test "x$PKG_CONFIG" != "x" ; then -PKG_CHECK_MODULES(LIBPARTED, libparted >= $PARTED_REQUIRED, [], [PARTED_FOUND=no]) + if test "$PARTED_FOUND" = yes && test "x$PKG_CONFIG" != "x" ; then +PKG_CHECK_MODULES(LIBPARTED, libparted >= $PARTED_REQUIRED, [], + [PARTED_FOUND=no]) fi if test "$PARTED_FOUND" = "no"; then # RHEL-5 vintage parted is missing pkg-config files @@ -1739,8 +1740,10 @@ if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; the fi if test "$with_storage_disk" = "yes"; then -AC_DEFINE_UNQUOTED([WITH_STORAGE_DISK], 1, [whether Disk backend for storage driver is enabled]) -AC_DEFINE_UNQUOTED([PARTED],["$PARTED"], [Location or name of the parted program]) +AC_DEFINE_UNQUOTED([WITH_STORAGE_DISK], 1, + [whether Disk backend for storage driver is enabled]) +AC_DEFINE_UNQUOTED([PARTED],["$PARTED"], + [Location or name of the parted program]) fi fi AM_CONDITIONAL([WITH_STORAGE_DISK], [test "$with_storage_disk" = "yes"]) -- 1.7.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Allow to delete device mapper disk partition
On 01/31/2011 03:16 AM, Osier Yang wrote: > From: root Fix your ~/.gitconfig, then 'git commit --amend --author=Osier' to reclaim authorship of this file; this patch is showing up with the wrong author information, and even though we could .mailmap around a bad commit, I'd rather not have bogus commit information in the first place. > > This patch introduces 'dmsetup' to fix it. How portable is dmsetup to other Linux distros? (Well, at least Fedora 14 and Ubuntu 10.10 had it without me doing anything, so I'm a bit reassured). > diff --git a/configure.ac b/configure.ac > index f310a5e..b101faa 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1705,6 +1705,8 @@ LIBPARTED_CFLAGS= > LIBPARTED_LIBS= > if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; > then >AC_PATH_PROG([PARTED], [parted], [], [$PATH:/sbin:/usr/sbin]) > + AC_PATH_PROG([DMSETUP], [dmsetup], [], [$PATH:/sbin:/usr/sbin]) At least you aren't making things any worse - the storage manager already depended on external programs only likely to be found on Linux. > + >if test -z "$PARTED" ; then > with_storage_disk=no > PARTED_FOUND=no Hmm, pre-existing bug. If $with_storage_disk is yes, but $PARTED is not found, then we slam $with_storage_disk to no... > @@ -1712,9 +1714,17 @@ if test "$with_storage_disk" = "yes" || test > "$with_storage_disk" = "check"; the > PARTED_FOUND=yes >fi > > + if test -z "$DMSETUP" ; then > +with_storage_disk=no > +DMSETUP_FOUND=no > + else > +DMSETUP_FOUND=yes > + fi > + >if test "$with_storage_disk" != "no" && test "x$PKG_CONFIG" != "x" ; then > PKG_CHECK_MODULES(LIBPARTED, libparted >= $PARTED_REQUIRED, [], > [PARTED_FOUND=no]) >fi > + >if test "$PARTED_FOUND" = "no"; then > # RHEL-5 vintage parted is missing pkg-config files > save_LIBS="$LIBS" > @@ -1738,9 +1748,20 @@ if test "$with_storage_disk" = "yes" || test > "$with_storage_disk" = "check"; the > with_storage_disk=yes >fi > > + if test "$DMSETUP_FOUND" = "no"; then > +if test "$with_storage_disk" = "yes" ; then > + AC_MSG_ERROR([We need dmsetup for disk storage driver]) but then later check if PARTED_FOUND is no (as copied by you to DMSETUP_FOUND begin no), we are still looking for with_storage_disk to be yes, which it can't be. Let's fix that logic bug first, before you copy and paste it to occur twice. > +++ b/src/libvirt_private.syms > @@ -900,6 +900,7 @@ virStrcpy; > virStrncpy; > virTimestamp; > virVasprintf; > +virIsDeviceMapperDevice; Keep this list sorted. This new line should be between virIndexToDiskName and virKillProcess. > +} else { > +const char *prog[] = { > +DMSETUP, > +"remove", > +"-f", > +devpath, > +NULL, > +NULL, > +}; > + > +if (virRun(prog, NULL) < 0) We probably ought to switch these over to virCommand, as long as we're touching the code. > + > +int > +virIsDeviceMapperDevice(const char *devname) > +{ > +struct stat buf; > + > +if (devname && !stat(devname, &buf) && > dm_is_dm_major(major(buf.st_rdev))) { Phew - I was about to panic about licensing concerns, but it looks like you're okay - while the devmapper package consists of both LPGLv2 and GPLv2 portions, further reading shows that it is dmsetup that is GPLv2 (and external execution is fine), while the library is LGPLv2 (so moving the call to dm_* out of parthelper into the primary libvirt library is acceptable). > +int virIsDeviceMapperDevice(const char *devname); s/int/bool/ as long as you are renaming the function. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] qemu: Build command line for incoming tunneled migration
On 01/30/2011 12:58 AM, Osier Yang wrote: > Command line building for imcoming tunneled migration is missed, s/imcoming/incoming/ > as a result, all the tunneled migration will fail with "unknown > migration protocol". > > * src/qemu/qemu_command.c > --- > src/qemu/qemu_command.c |8 > 1 files changed, 8 insertions(+), 0 deletions(-) ACK - this is a regression that I introduced in commit 1859939a (prior to that point, the code validated tcp, stdio, and exec in one place in the code, then blindly used the string in another without regards to it being a possible fourth value; the rewrite rejected unknown values, which is incorrect). Please apply! -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] qemu: Build command line for incoming tunneled migration
On 01/30/2011 01:10 AM, Osier Yang wrote: > This patch is race with: > http://www.redhat.com/archives/libvir-list/2011-January/msg00681.html > > , if patch above works, but it was not pushed yet, will it be > pushed? I've been kind of sitting on that patch - it's an optimization rather than a correctness issue, and Paolo correctly pointed out that we can further optimize by also teaching the outgoing side to pass an fd to the monitor (since the monitor connection is a Unix socket) rather than the current exec: used on the outgoing side. Since I have only written the -incoming side so far, I've deferred pushing until the outgoing side is also written. I'm okay if yours goes in first. Meanwhile, both your patch and my (deferred) patch conflict with Daniel's refactoring into qemu_process and qemu_migration. Fun stuff, merge conflicts :) -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Move all the QEMU migration code to a new file
On 01/31/2011 04:06 AM, Daniel P. Berrange wrote: >>> The functions are not moved 100%. The API entry points >>> remain in the main QEMU driver, but once the public >>> virDomainPtr is resolved to the internal virDomainObjPtr, >>> all following code is moved. >>> >>> This will allow the new v3 API entry points to call into the >>> same shared internal migration functions >>> >> The diffstat makes this look like a cleaner diff than the qemu_process >> split, so it should be easier to review. >> >>> - qemu/qemu_process.c qemu/qemu_process.h \ >>> + qemu/qemu_process.c qemu/qemu_process.h \ >>> + qemu/qemu_migration.c qemu/qemu_migration.h \ >> >> However, this means that you depend on the qemu_process split to happen >> first, and that one had issues, so I'm not reviewing this one yet (but I >> do like the idea of the patch). > > The 2 line fix I posted to the other patch shouldn't > invalidate this patch Agreed, so I'm reviewing it now... Another case of a missing po/POTFILES.in tweak to keep 'syntax-check' happy. > +++ b/src/Makefile.am > @@ -281,7 +281,8 @@ QEMU_DRIVER_SOURCES = > \ > qemu/qemu_hostdev.c qemu/qemu_hostdev.h \ > qemu/qemu_hotplug.c qemu/qemu_hotplug.h \ > qemu/qemu_conf.c qemu/qemu_conf.h \ > - qemu/qemu_process.c qemu/qemu_process.h \ > + qemu/qemu_process.c qemu/qemu_process.h \ Is it worth squashing this whitespace tweak into the prior patch, > + qemu/qemu_migration.c qemu/qemu_migration.h \ so that this commit remains just the new file? > + > + > +char *qemuDomainFormatXML(struct qemud_driver *driver, > + virDomainObjPtr vm, > + int flags) > +{ > + > +if (!(cpu = virCPUDefCopy(def_cpu)) > +|| cpuUpdate(cpu, driver->caps->host.cpu)) Unusual formatting (generally, the || is on the previous line). > > -static char *qemudVMDumpXML(struct qemud_driver *driver, > -virDomainObjPtr vm, > -int flags) > -{ > -if (!(cpu = virCPUDefCopy(def_cpu)) > -|| cpuUpdate(cpu, driver->caps->host.cpu)) Oh, I see - code motion. Also need a single line tweak for cppi, but it passed testing for me, and was easier to review than the first. ACK with nits (aka syntax-check) fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] [PATCHv4] bridge_driver: handle DNS over IPv6
* dnsmasq listens on all defined IPv[46] addresses for network * Add ip6tables rules to allow DNS traffic to host --- src/network/bridge_driver.c | 51 ++ 1 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c098ab5..24be0b7 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -432,6 +432,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; char *bridgeaddr; +int ii; +virNetworkIpDefPtr tmpipdef; if (!(bridgeaddr = virSocketFormatAddr(&ipdef->address))) goto cleanup; @@ -468,20 +470,28 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, /* *no* conf file */ virCommandAddArgList(cmd, "--conf-file=", "", NULL); -/* - * XXX does not actually work, due to some kind of - * race condition setting up ipv6 addresses on the - * interface. A sleep(10) makes it work, but that's - * clearly not practical - * - * virCommandAddArg(cmd, "--interface"); - * virCommandAddArg(cmd, ipdef->bridge); - */ virCommandAddArgList(cmd, - "--listen-address", bridgeaddr, "--except-interface", "lo", NULL); +/* + * --interface does not actually work with dnsmasq < 2.47, + * due to DAD for ipv6 addresses on the interface. + * + * virCommandAddArgList(cmd, "--interface", ipdef->bridge, NULL); + * + * So listen on all defined IPv[46] addresses + */ +for (ii = 0; + (tmpipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); + ii++) { + char *ipaddr = virSocketFormatAddr(&tmpipdef->address); +if (!ipaddr) +goto cleanup; +virCommandAddArgList(cmd, "--listen-address", ipaddr, NULL); +VIR_FREE(ipaddr); +} + for (r = 0 ; r < ipdef->nranges ; r++) { char *saddr = virSocketFormatAddr(&ipdef->ranges[r].start); if (!saddr) @@ -1027,9 +1037,30 @@ networkAddGeneralIp6tablesRules(struct network_driver *driver, goto err3; } +/* allow DNS over IPv6 */ +if (iptablesAddTcpInput(driver->iptables, AF_INET6, +network->def->bridge, 53) < 0) { +networkReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to add ip6tables rule to allow DNS requests from '%s'"), + network->def->bridge); +goto err4; +} + +if (iptablesAddUdpInput(driver->iptables, AF_INET6, +network->def->bridge, 53) < 0) { +networkReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to add ip6tables rule to allow DNS requests from '%s'"), + network->def->bridge); +goto err5; +} + return 0; /* unwind in reverse order from the point of failure */ +err5: +iptablesRemoveTcpInput(driver->iptables, AF_INET6, network->def->bridge, 53); +err4: +iptablesRemoveForwardAllowCross(driver->iptables, AF_INET6, network->def->bridge); err3: iptablesRemoveForwardRejectIn(driver->iptables, AF_INET6, network->def->bridge); err2: -- 1.7.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Raising events for testing
Yep that's what I've just seen. I've decided to try via mingwin which seems to implement a poll function...Callbacks implementation are really hard for a noob :) Regards, Arnaud De: "Eric Blake" Envoyé: lundi 31 janvier 2011 20:12 A: arnaud.champ...@si-vs1281.com Objet: Re: [libvirt] Raising events for testing On 01/31/2011 11:52 AM, arnaud.champ...@devatom.fr wrote: >> There is - it involves using gnulib. But that is only for C and C++ - I >> have no idea how this translates over to C# > So if I understand well, are you saying that if I use gnulib windows version > via C# (pinvoke, this kind of magic) I will have access to poll function ? > Intersting, I'll take a look. I hope gnulib if published via a dll under > windows, if so, I'm pretty sure I can use it with C# > regards Sorry; gnulib is a source only library that currently targets only C and C++; it does not target C#, and does not provide any dll, so I have no idea how that affects your porting efforts. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Raising events for testing
On 01/31/2011 11:52 AM, arnaud.champ...@devatom.fr wrote: >> There is - it involves using gnulib. But that is only for C and C++ - I >> have no idea how this translates over to C# > So if I understand well, are you saying that if I use gnulib windows version > via C# (pinvoke, this kind of magic) I will have access to poll function ? > Intersting, I'll take a look. I hope gnulib if published via a dll under > windows, if so, I'm pretty sure I can use it with C# > regards Sorry; gnulib is a source only library that currently targets only C and C++; it does not target C#, and does not provide any dll, so I have no idea how that affects your porting efforts. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Split all QEMU process mangement code into separate file
On 01/31/2011 04:05 AM, Daniel P. Berrange wrote: >>> - >>> -if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0) >>> -goto cleanup; >> >> Why was the SetCurrentInactive line commented out in the move? > > It is already invoked in the caller, so it should be redundant > here, and it causes a circular dependency if we leave it in. Makes sense; do you want to just nuke the #if 0 chunk, then? >>> +if (virDomainObjUnref(obj) > 0) { >>> +/* We can't get the monitor back, so must kill the VM >>> + * to remove danger of it ending up running twice if >>> + * user tries to start it again later */ >>> +qemudShutdownVMDaemon(driver, obj, 0); >> >> Wouldn't this cause a compile error, since you renamed the function to >> qemuProcessStop? Indeed: > This is a result of rebasing just before posting it. The fixes > are trivial Yep, which leaves only the po/POTFILES.in, cppi cleanups, and removal of '#include "c-ctype.h"' from qemu_driver, and the trailing empty line in qemu_process.c, (for 'make syntax-check') as the only remaining nits, also trivial to fix. ACK with nits fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Raising events for testing
>There is - it involves using gnulib. But that is only for C and C++ - I >have no idea how this translates over to C# So if I understand well, are you saying that if I use gnulib windows version via C# (pinvoke, this kind of magic) I will have access to poll function ? Intersting, I'll take a look. I hope gnulib if published via a dll under windows, if so, I'm pretty sure I can use it with C# regards Arnaud De: "Eric Blake" Envoyé: lundi 31 janvier 2011 19:37 A: arnaud.champ...@devatom.fr Objet: Re: [libvirt] Raising events for testing On 01/30/2011 05:21 AM, arnaud.champ...@devatom.fr wrote: > Hi, > > One thing I need to understand, in the callback python or c samples, the > fileDescriptor is on a socket or on a pipe ? Libvirt uses gnulib to guarantee that poll() works on file descriptors; all socket operations are magically mapped by gnulib to behave like file descriptors, so that you don't have to worry about the (less-useful) restrictions of windows poll() only working on sockets. >> Okay I have understand the use and mean of poll (pollfd... >> bla...bla...bla...) >> I have also discover that there is no Win32 equivalent which work with >> a file descriptor (unix way...). There is - it involves using gnulib. But that is only for C and C++ - I have no idea how this translates over to C#. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] interface description
On 01/31/2011 12:10 PM, arnaud.champ...@devatom.fr wrote: Hi, I have an interface xml description the "model" is missing, is that normal ? if it is normal, what is the exposed model ? It's not required to specify in the XML. If you don't give a model, it looks like it defaults to rtl8139 (see qemuBuildNicDevStr()). ** ** ** **type="*pci*"domain="*0x*"bus="*0x00*"slot="*0x03*"function="*0x0*"/> Regards Arnaud -- 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] tests: handle backspace-newline pairs in test input files
On 01/31/2011 05:24 AM, Haefliger, Juerg wrote: >>From 04856f4c830a5f40d44bb67e908a1cbf88c18ce5 Mon Sep 17 00:00:00 2001 > From: Juerg Haefliger > Date: Mon, 31 Jan 2011 06:42:57 -0500 > Subject: [PATCH] tests: handle backspace-newline pairs in test input files > > This patch teaches testutil how to read multi-line input files with > backspace-newline line continuation markers. Thanks for tackling this! > > The patch also breaks up all the single-line arguments test input files into > multi-line files with lines shorther than 80 characters. s/shorther/shorter/ > --- > > Not sure if my mailer handles the long lines properly so I'm attaching the > patch file as well, just in case. Probably wise (I used just the attachment, given that the whole point of this exercise is getting rid of mailer problems, but they aren't gone until after this patch is in :) Your patch failed 'make syntax-check': TAB_in_indentation tests/testutils.c:211: /* advance the temporary buffer pointer */ maint.mk: use spaces, not TAB, for indentation in C, sh, and RNG schemas Also, I have a (minor) concern that there may be cases where we WANT to preserve a literal backslash-newline in some test files, rather than flattening it for all clients. That is, would it be better to add a new function virtTestLoadFileFlags with a flag argument that requests whether to do \\-\n flattening, and adjust callers that need it while keeping the existing virtTestLoadFile as a simple wrapper to virtTestLoadFileFlags(,0)? But as none of tests failed, that means none of our tests are affected by this, so I'm okay with the patch as-is (and in fact, that means if we add a test in the future that _does_ care about literal \\-\n, then we would would add virtTestLoadFileFlags with a non-zero to preserve literals, and not have to change any of the existing callers that get flattening by default). > @@ -197,17 +199,28 @@ int virtTestLoadFile(const char *file, > return -1; > } > > +(*buf)[0] = '\0'; > if (st.st_size) { > -if (fread(*buf, st.st_size, 1, fp) != 1) { > +/* read the file line by line */ > +while (fgets(tmp, tmplen, fp) != NULL) { > +len = strlen(tmp); > +/* remove trailing backslash-newline pair */ > +if (len >= 2 && tmp[len-2] == '\\' && tmp[len-1] == '\n') { > +len -= 2; > +} > + /* advance the temporary buffer pointer */ > +tmp += len; > +tmplen -= len; > +} > +if (ferror(fp)) { > fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); > VIR_FORCE_FCLOSE(fp); > return -1; > } > } > -(*buf)[st.st_size] = '\0'; > > VIR_FORCE_FCLOSE(fp); > -return st.st_size; > +return strlen(*buf); Mishandles any embedded NUL bytes, but again, none of our tests currently use embedded NULs, so we could add a flag in the future to force being more careful with a binary file if we have a reason to slurp in a binary file. Mishandles files that end in the two bytes backslash and newline (you back up the pointer by two, but the next fgets() returns NULL because it is at EOF without writing a NUL at the start location) - but that's trivially solvable (I squashed in tmp[len] = '\0' just after the len -= 2 line). And not optimal - it counts the string length twice (once while reading, and once at the end), but that's still O(n) and not worth the hassle of refactoring things just to keep an accurate running total while reading lines. I like it enough as is, that I'm comfortable giving: ACK with the syntax-check nit fixed So I pushed it. Thanks again for taking on my request: https://www.redhat.com/archives/libvir-list/2011-January/msg01149.html -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] Raising events for testing
On Sun, Jan 30, 2011 at 01:21:37PM +0100, arnaud.champ...@devatom.fr wrote: > Hi, > > One thing I need to understand, in the callback python or c samples, > the fileDescriptor is on a socket or on a pipe ? Internally libvirt may have a opened pipe, file, socket, or something else. In all cases, the 'fd' parameter that is passed to callbacks is what Microsoft call "a C runtime file descriptor". In particular what you get in a callback is *not* a HANDLE or SOCKET object, even if the underlying object libvirt has internally is a SOCKET/HANDLE. Basically the libvirt code on Windows that creates sockets does SOCKET s = WSASocket(...) int fd = _open_osfhandle(s); The callback gets 'fd', and not 's'. Regard, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Raising events for testing
On 01/30/2011 05:21 AM, arnaud.champ...@devatom.fr wrote: > Hi, > > One thing I need to understand, in the callback python or c samples, the > fileDescriptor is on a socket or on a pipe ? Libvirt uses gnulib to guarantee that poll() works on file descriptors; all socket operations are magically mapped by gnulib to behave like file descriptors, so that you don't have to worry about the (less-useful) restrictions of windows poll() only working on sockets. >> Okay I have understand the use and mean of poll (pollfd... >> bla...bla...bla...) >> I have also discover that there is no Win32 equivalent which work with >> a file descriptor (unix way...). There is - it involves using gnulib. But that is only for C and C++ - I have no idea how this translates over to C#. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] docs: update docs for cpu_shares setting
On 01/29/2011 10:43 PM, Osier Yang wrote: > * tools/virsh.pod > --- > tools/virsh.pod |6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/tools/virsh.pod b/tools/virsh.pod > index 441ca8a..d3262b6 100644 > --- a/tools/virsh.pod > +++ b/tools/virsh.pod > @@ -566,7 +566,11 @@ Xen (credit scheduler): weight, cap > > ESX (allocation scheduler): reservation, limit, shares > > -B: The cpu_shares parameter has a valid value range of 0-262144. > +B: The cpu_shares parameter has a valid value range of 0-262144, > however, > +negative number and positive number greater than 262144 are allowed, but > +negative number will be converted into positive number, and positive number > +greater than 262144 will be reduced to 262144 by cgroup in the end, e.g. -1 > +can be used as useful shorthand for 262144. The range of 262144 is specific to cgroups (qemu and lxc); it is possible that some other hypervisor may add similar support for this parameter, but have a different range. Then again, we already documented it is specific to qemu and lxc, so we could update a change in range at the time of introducing the cpu_shares parameter to another hypervisor. However, your attempt is still wordy. Can we go with something more compact, as in: B: The cpu_shares parameter has a valid value range of 0-262144; negative values are wrapped to positive, and larger values are capped at the maximum. Therefore, -1 is a useful shorthand for 262144. -- Eric Blake ebl...@redhat.com+1-801-349-2682 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] interface description
?Hi, I have an interface xml description the "model" is missing, is that normal ? if it is normal, what is the exposed model ? Regards Arnaud-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: More clear error parsing domain def failure of tunneled migration
于 2011年01月31日 18:50, Daniel P. Berrange 写道: On Sun, Jan 30, 2011 at 04:46:39PM +0800, Osier Yang wrote: * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 84d339b..929dc94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8014,7 +8014,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(def = virDomainDefParseString(driver->caps, dom_xml, VIR_DOMAIN_XML_INACTIVE))) { qemuReportError(VIR_ERR_OPERATION_FAILED, -"%s", _("failed to parse XML")); +"%s", _("failed to parse XML, libvirt version may be " +"diffrent between source and destination host")); goto cleanup; NACK, this code is already broken. The virDomainDefParseString() method *already* reported the real problem. This call to qemuReportError() is overwriting the real error message with something that is useless. The qemuReportError() call needs to just be deleted entirely. Oops, As it's already pushed, so will make another patch to remove qemuReportError(), Thanks. Regards Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] fork of php-libvirt
Guys, in fact I did a fork of PHP libvirt and I informed Radek as well, my fork can be found on github at my profile page: https://github.com/MigNov/php-libvirt Also, sorry for having the root in some commits but I used wrong user-name with git unset to merge it. I've been testing it in my spare time on Fedora 14 and php-5.3.4-1.fc14.1.i686 . Michal On 01/27/2011 03:05 AM, Lyre wrote: Hi all: I've added the documentation for the new APIs several days ago, but I've been busy these days and forget to inform Radek. So Radek, you may need to rebuild the documents. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Michal Novotny, RHCE Virtualization Team (xen userspace), Red Hat -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] callback question
Okay, I understand. When the AddHandle function, I receive a fd (FileDescription), its value is 3... always... Is that normal ? Is that file descriptor on a socket or on a file ? (In windows, when can use the WSAPoll function but it works on ly on sockets) Regards, Arnaud -- From: "Daniel P. Berrange" Sent: Monday, January 31, 2011 12:43 PM To: Cc: Subject: Re: [libvirt] callback question On Mon, Jan 31, 2011 at 12:16:24PM +0100, arnaud.champ...@devatom.fr wrote: ?Hi, Something I need to understant about callbacks: if I install 2 callback via DomainEventRegisterAny (lifecycle for the first and graphics for the second for example), am I right if I said that the AddHandleFunc should be called 2 times ? No, there is no direct relationship between the number of calls to DomainEventRegister* and AddHandleFunc. In the current impl we only call AddHandleFunc once per virConnect object, but again that isn't guarenteed - different libvirt drivers will invoke AddHandleFunc / AddTimeoutFunc a different number of times, as is most suitable for their precise needs. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] callback question
On Mon, Jan 31, 2011 at 12:16:24PM +0100, arnaud.champ...@devatom.fr wrote: > ?Hi, > > Something I need to understant about callbacks: > > if I install 2 callback via DomainEventRegisterAny (lifecycle > for the first and graphics for the second for example), am I > right if I said that the AddHandleFunc should be called 2 times ? No, there is no direct relationship between the number of calls to DomainEventRegister* and AddHandleFunc. In the current impl we only call AddHandleFunc once per virConnect object, but again that isn't guarenteed - different libvirt drivers will invoke AddHandleFunc / AddTimeoutFunc a different number of times, as is most suitable for their precise needs. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Google Summer of Code 2011
On Sun, 30 Jan 2011 16:06:20 +0100 Alexander Graf wrote: > > On 28.01.2011, at 21:10, Luiz Capitulino wrote: > > > Hi there, > > > > GSoC 2011 has been announced[1]. As we were pretty successful last year, > > I think we should participate again. I've already created a wiki page: > > > > http://wiki.qemu.org/Google_Summer_of_Code_2011 > > > > We should now populate it with projects and people willing to be mentors > > should say so (or just add a project)[2]. > > > > Also, I'd like to do something different this year, I'd like to invite > > libvirt people to join. There are two ways of doing this: > > > > 1. They join in the program as a regular mentoring organization, or > > > > 2. They join with QEMU > > > > The second option means that libvirt can suggest and run its own projects > > (preferably with QEMU relevance), but from a GSoC perspective, the project > > will be part of the QEMU org. > > Keep in mind that every full org gets a free trip to the west coast for 2 > people ;). So splitting up means we could almost do a mini-summit at the > google campus on google's expenses ;). Actually, they have a limited budget and if you live too far (say, in Brazil), the trip might not be 100% free :) > Please coordinate that with Carol. Apparently traction for GSOC is declining > (according to last year's summit). So there might be plenty of available > slots this year. So I'd say sign up separately for now and if you don't get > accepted, just join forces with us! Yes, that's a good plan and I fully agree that we get more benefits if we apply separately. It's a call to libvirt's people. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] callback question
?Hi, Something I need to understant about callbacks: if I install 2 callback via DomainEventRegisterAny (lifecycle for the first and graphics for the second for example), am I right if I said that the AddHandleFunc should be called 2 times ? Regards, Arnaud-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Move all the QEMU migration code to a new file
On Sat, Jan 29, 2011 at 11:44:39AM -0700, Eric Blake wrote: > On 01/28/2011 06:22 AM, Daniel P. Berrange wrote: > > The introduction of the v3 migration protocol, along with > > support for migration cookies, will significantly expand > > the size of the migration code. Move it all to a separate > > file to make it more manageable > > > > The functions are not moved 100%. The API entry points > > remain in the main QEMU driver, but once the public > > virDomainPtr is resolved to the internal virDomainObjPtr, > > all following code is moved. > > > > This will allow the new v3 API entry points to call into the > > same shared internal migration functions > > > > * src/qemu/qemu_domain.c, src/qemu/qemu_domain.h: Add > > qemuDomainFormatXML helper method > > * src/qemu/qemu_driver.c: Remove all migration code > > * src/qemu/qemu_migration.c, src/qemu/qemu_migration.h: Add > > all migration code. > > --- > > src/Makefile.am |3 +- > > src/qemu/qemu_domain.c| 39 ++ > > src/qemu/qemu_domain.h|4 + > > src/qemu/qemu_driver.c| 1296 > > ++--- > > src/qemu/qemu_migration.c | 1295 > > > > src/qemu/qemu_migration.h | 63 +++ > > 6 files changed, 1444 insertions(+), 1256 deletions(-) > > create mode 100644 src/qemu/qemu_migration.c > > create mode 100644 src/qemu/qemu_migration.h > > The diffstat makes this look like a cleaner diff than the qemu_process > split, so it should be easier to review. > > > - qemu/qemu_process.c qemu/qemu_process.h \ > > + qemu/qemu_process.c qemu/qemu_process.h \ > > + qemu/qemu_migration.c qemu/qemu_migration.h \ > > However, this means that you depend on the qemu_process split to happen > first, and that one had issues, so I'm not reviewing this one yet (but I > do like the idea of the patch). The 2 line fix I posted to the other patch shouldn't invalidate this patch Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Split all QEMU process mangement code into separate file
On Sat, Jan 29, 2011 at 11:41:33AM -0700, Eric Blake wrote: > On 01/28/2011 06:21 AM, Daniel P. Berrange wrote: > > -static int qemudStartVMDaemon(virConnectPtr conn, > > - struct qemud_driver *driver, > ... > > > -if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, > > priv->monConfig, > > - priv->monJSON != 0, qemuCmdFlags, > > - migrateFrom, stdin_fd, > > - vm->current_snapshot, vmop))) > > -goto cleanup; > > - > > -if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0) > > -goto cleanup; > > Why was the SetCurrentInactive line commented out in the move? It is already invoked in the caller, so it should be redundant here, and it causes a circular dependency if we leave it in. > > +static void > > +qemuProcessReconnect(void *payload, const char *name ATTRIBUTE_UNUSED, > > void *opaque) > > +{ > > > +if (virDomainObjUnref(obj) > 0) { > > +/* We can't get the monitor back, so must kill the VM > > + * to remove danger of it ending up running twice if > > + * user tries to start it again later */ > > +qemudShutdownVMDaemon(driver, obj, 0); > > Wouldn't this cause a compile error, since you renamed the function to > qemuProcessStop? Indeed: > > cc1: warnings being treated as errors > qemu/qemu_process.c: In function 'qemuProcessReconnect': > qemu/qemu_process.c:1854:9: error: implicit declaration of function > 'qemudShutdownVMDaemon' [-Wimplicit-function-declaration] > qemu/qemu_process.c:1854:9: error: nested extern declaration of > 'qemudShutdownVMDaemon' [-Wnested-externs] > qemu/qemu_process.c: In function 'qemuProcessStart': > qemu/qemu_process.c:1949:9: error: implicit declaration of function > 'fstat' [-Wimplicit-function-declaration] > qemu/qemu_process.c:1949:9: error: nested extern declaration of 'fstat' > [-Wnested-externs] > qemu/qemu_process.c:1954:9: error: implicit declaration of function > 'S_ISFIFO' [-Wimplicit-function-declaration] > qemu/qemu_process.c:1954:9: error: nested extern declaration of > 'S_ISFIFO' [-Wnested-externs] This is a result of rebasing just before posting it. The fixes are trivial diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1e3dea2..76c48dd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "qemu_process.h" #include "qemu_domain.h" @@ -1851,7 +1852,7 @@ error: /* We can't get the monitor back, so must kill the VM * to remove danger of it ending up running twice if * user tries to start it again later */ -qemudShutdownVMDaemon(driver, obj, 0); +qemuProcessStop(driver, obj, 0); if (!obj->persistent) virDomainRemoveInactive(&driver->domains, obj); else Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: More clear error parsing domain def failure of tunneled migration
On Sun, Jan 30, 2011 at 04:46:39PM +0800, Osier Yang wrote: > * src/qemu/qemu_driver.c > --- > src/qemu/qemu_driver.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 84d339b..929dc94 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -8014,7 +8014,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, > if (!(def = virDomainDefParseString(driver->caps, dom_xml, > VIR_DOMAIN_XML_INACTIVE))) { > qemuReportError(VIR_ERR_OPERATION_FAILED, > -"%s", _("failed to parse XML")); > +"%s", _("failed to parse XML, libvirt version may be > " > +"diffrent between source and destination > host")); > goto cleanup; NACK, this code is already broken. The virDomainDefParseString() method *already* reported the real problem. This call to qemuReportError() is overwriting the real error message with something that is useless. The qemuReportError() call needs to just be deleted entirely. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: Allow to delete device mapper disk partition
From: root The name convention of device mapper disk is different, and 'parted' can't be used to delete a device mapper disk partition. e.g. [root@amd-1352-8-2 ~]# virsh vol-list --pool osier Name Path - 3600a0b80005ad1d793604cae912fp1 /dev/mapper/3600a0b80005ad1d793604cae912fp1 [root@amd-1352-8-2 ~]# parted /dev/mapper/3600a0b80005adb0bab2d4cae9254 rm p1 Error: Expecting a partition number. This patch introduces 'dmsetup' to fix it. Changes: - New function 'virIsDeviceMapperDevice' in src/utils/utils.c - Delete 'is_dm_device' in src/storage/parthelper.c, use 'virIsDeviceMapperDevice' instead. - Requires device-mapper for 'with-storage-disk' in libvirt.spec.in - Check 'dmsetup' in configure.ac for 'with-storage-disk' - Changes on src/Makefile.am to link against libdevmapper - New entry for 'virIsDeviceMapperDevice' in src/libvirt_private.syms e.g. [root@amd-1352- ~]# virsh vol-list --pool osier Name Path - 3600a0b80005ad1d793604cae912fp1 /dev/mapper/3600a0b80005ad1d793604cae912fp1 [root@amd-1352- ~]# virsh vol-delete /dev/mapper/3600a0b80005ad1d793604cae912fp1 Vol /dev/mapper/3600a0b80005ad1d793604cae912fp1 deleted [root@amd-1352- ~]# virsh vol-list --pool osier Name Path - --- configure.ac | 21 ++ libvirt.spec.in|1 + src/Makefile.am|7 ++-- src/libvirt_private.syms |1 + src/storage/parthelper.c | 14 + src/storage/storage_backend_disk.c | 54 ++- src/util/util.c| 14 + src/util/util.h|1 + 8 files changed, 76 insertions(+), 37 deletions(-) diff --git a/configure.ac b/configure.ac index f310a5e..b101faa 100644 --- a/configure.ac +++ b/configure.ac @@ -1705,6 +1705,8 @@ LIBPARTED_CFLAGS= LIBPARTED_LIBS= if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; then AC_PATH_PROG([PARTED], [parted], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([DMSETUP], [dmsetup], [], [$PATH:/sbin:/usr/sbin]) + if test -z "$PARTED" ; then with_storage_disk=no PARTED_FOUND=no @@ -1712,9 +1714,17 @@ if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; the PARTED_FOUND=yes fi + if test -z "$DMSETUP" ; then +with_storage_disk=no +DMSETUP_FOUND=no + else +DMSETUP_FOUND=yes + fi + if test "$with_storage_disk" != "no" && test "x$PKG_CONFIG" != "x" ; then PKG_CHECK_MODULES(LIBPARTED, libparted >= $PARTED_REQUIRED, [], [PARTED_FOUND=no]) fi + if test "$PARTED_FOUND" = "no"; then # RHEL-5 vintage parted is missing pkg-config files save_LIBS="$LIBS" @@ -1738,9 +1748,20 @@ if test "$with_storage_disk" = "yes" || test "$with_storage_disk" = "check"; the with_storage_disk=yes fi + if test "$DMSETUP_FOUND" = "no"; then +if test "$with_storage_disk" = "yes" ; then + AC_MSG_ERROR([We need dmsetup for disk storage driver]) +else + with_storage_disk=no +fi + else +with_storage_disk=yes + fi + if test "$with_storage_disk" = "yes"; then AC_DEFINE_UNQUOTED([WITH_STORAGE_DISK], 1, [whether Disk backend for storage driver is enabled]) AC_DEFINE_UNQUOTED([PARTED],["$PARTED"], [Location or name of the parted program]) +AC_DEFINE_UNQUOTED([DMSETUP],["$DMSETUP"], [Location or name of the dmsetup program]) fi fi AM_CONDITIONAL([WITH_STORAGE_DISK], [test "$with_storage_disk" = "yes"]) diff --git a/libvirt.spec.in b/libvirt.spec.in index 0a2d10e..27e6a99 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -279,6 +279,7 @@ Requires: iscsi-initiator-utils %if %{with_storage_disk} # For disk driver Requires: parted +Requires: device-mapper %endif %if %{with_storage_mpath} # For multipath support diff --git a/src/Makefile.am b/src/Makefile.am index f8b8434..3680074 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -437,9 +437,9 @@ libvirt_la_BUILT_LIBADD = libvirt_util.la libvirt_util_la_SOURCES = \ $(UTIL_SOURCES) libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ - $(AM_CFLAGS) $(AUDIT_CFLAGS) + $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_LIBS) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ - $(LIB_PTHREAD) $(AUDIT_LIBS) + $(LIB_PTHREAD) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) noinst_LTLIBRARIES += libvirt_conf.la @@ -1154,7 +1154,6 @@ libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) libvirt_parthelper_LDADD = \ $(LIBPARTED_LIBS) \ - $(DEVMAPPER_LIBS) \ libv
Re: [libvirt] [PATCH] qemu: More clear error parsing domain def failure of tunneled migration
于 2011年01月31日 15:39, Daniel Veillard 写道: On Mon, Jan 31, 2011 at 12:52:55PM +0800, Osier Yang wrote: * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 84d339b..929dc94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8014,7 +8014,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, if (!(def = virDomainDefParseString(driver->caps, dom_xml, VIR_DOMAIN_XML_INACTIVE))) { qemuReportError(VIR_ERR_OPERATION_FAILED, -"%s", _("failed to parse XML")); +"%s", _("failed to parse XML, libvirt version may be " +"different between source and destination host")); goto cleanup; } ACK, please push :-) Thanks, pushed. :-) Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] init cont->idx correctly
On Mon, Jan 31, 2011 at 02:15:41PM +0800, Wen Congyang wrote: > Steps to reproduce this bug: > 1. # virsh attach-disk --target sdb ... > 2. # virsh attach-disk --target sdh ... >error: Failed to attach disk >error: operation failed: target scsi:0 already exists > > sdh uses scsi:1, rather than scsi:0. > > Signed-off-by: Wen Congyang > > --- > src/qemu/qemu_hotplug.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index c334f52..670e7d3 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -321,7 +321,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct > qemud_driver *driver, > return NULL; > } > cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; > -cont->idx = 0; > +cont->idx = controller; > cont->model = -1; > > VIR_INFO0("No SCSI controller present, hotplugging one"); Okay, based on the earlier lookup code in that function that looks like the obvious fix, ACK, and pushed, 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