Re: [libvirt] [PATCH] build: use correct limit for unsigned long long
On Fri, Jun 29, 2012 at 03:11:41PM -0600, Eric Blake wrote: Reported by Jason Helfman as a build-breaker on FreeBSD. * src/conf/domain_conf.c (virDomainFSDefParseXML): Use POSIX spelling. * src/openvz/openvz_conf.c (openvzReadFSConf): Likewise. --- Pushing under the build-breaker rule. src/conf/domain_conf.c |8 +++- src/openvz/openvz_conf.c |4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4086dac..3fb90db 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4248,14 +4248,12 @@ virDomainFSDefParseXML(xmlNodePtr node, if (virDomainParseScaledValue(./space_hard_limit[1], ctxt, def-space_hard_limit, 1, - ULONG_LONG_MAX, - false) 0) + ULLONG_MAX, false) 0) goto error; if (virDomainParseScaledValue(./space_soft_limit[1], ctxt, def-space_soft_limit, 1, - ULONG_LONG_MAX, - false) 0) + ULLONG_MAX, false) 0) goto error; cur = node-children; @@ -4335,7 +4333,7 @@ virDomainFSDefParseXML(xmlNodePtr node, } if (unit virScaleInteger(def-usage, unit, -1024, ULONG_LONG_MAX) 0) +1024, ULLONG_MAX) 0) goto error; } diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index bb61b13..ad27d37 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -455,8 +455,8 @@ openvzReadFSConf(virDomainDefPtr def, goto error; } else { /* Ensure that we can multiply by 1024 without overflowing. */ -if (barrier ULONG_LONG_MAX / 1024 || -limit ULONG_LONG_MAX / 1024 ) { +if (barrier ULLONG_MAX / 1024 || +limit ULLONG_MAX / 1024 ) { virReportSystemError(VIR_ERR_OVERFLOW, _(%s), Unable to parse quota); Probably worth blacklisting the non-portable spelling with syntax-check 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] compile option: --without-gnutls
On Fri, Jun 29, 2012 at 06:09:53PM -0400, Chris Van Heuveln wrote: Hi libvirt-list, Full disclosure: this is the first open source project I've tried to contribute to and I'm still learning the process and tools. Welcome, and don't worry, we're a generally friendly bunch who like seeing new contributors :-) In any case, Mikhail Gusarov started a thread on this a while back and I've tried to take it to conclusion. I needed to remove tls for 0.8.4 so I coded up a --without-gnutls configure option using Mikhail's partial code as a template. Then I pulled down 0.9.12 and came up with a patch that I'd like to push out to the commununity. As a general rule we recommend that people do their work against the latest GIT repository, rather than a formal release. With the pace of libvirt code development, you can find your patches against formal releases, won't cleanly apply to the latest GIT. So using GIT directly avoids you that headache. As an added bonus, it means you can also use 'git send-email' to submit your patch to the mailing list, avoiding the risk of your email client mangling the whitespace ! I'm not adding any new functionality other than the configure option. The tls calls/structs are just #ifdef'd for the most part. In a couple of places I had to flip the logic for if (!tls_struct) else, and also had to modify a few function calls to pass void * instead of tls pointers. Ok, sounds like a reasonable way to start. I built --with-gnutls, --without-gnutls, and no option specified (defaults to --with-gnutls). make check is clean except for: TEST: libvirtdconftest .!!...! 39 FAIL ...which consists mostly of this sasl failure: 39) Test corruption ... libvir: Config File error : unsupported configuration: remoteReadConfigFile: /home/ubu/git/libvirt/tests/../daemon/libvirtd.conf: auth_tcp: unsupported auth sasl FAILED ...so I'm trying to track that down, and also need to run syntax-check and valgrind tests. When debugging the test cases, you can see more verbose output by setting VIR_TEST_DEBUG=1 before running it, eg cd tests VIR_TEST_DEBUG=1 ./libvirtdconftest Once I get the tests passing what's my next step? Are you okay with this approach? Do you want a preview of my diffs? Do I need to write additional tests for this new option or update any documentation? Historically, we've never bothered to document the configure script options - just rely on autoconf's automatic help text. So my recommendation is to just update your patches to apply against latest GIT master, and then send them to this list. 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] sanlock on F17
On Fri, Jun 29, 2012 at 08:06:34PM -0400, Dave Allan wrote: I just tried to set up sanlock on F17 using the instructions at http://libvirt.org/locking.html, but libvirtd refuses to start with the sanlock error in the logs: Jun 29 19:56:20 nienna sanlock[8423]: 19846 open error -13 /var/lib/libvirt/sanlock/__LIBVIRT__DISKS__ I confirmed that user sanlock can create, read and write files in /var/lib/libvirt/sanlock The only thing I can think of that's slightly odd about my setup is that I'm only concerned with the local machine, so that directory is not an NFS mount. Anybody have any thoughts on what's going wrong here? Well, -13 is the EACCESS errno, so there must be some kind of permissions problem in there, be it DAC or SELinux. You might try strace'ing sanlockd just to be sure, and check that it really is running under the user/group that you think it is. 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] libvirt can't get capablities
Hi, I have suffered several times on both x86 machine and power machine. When we want to use libvirt to create on VMs, it reports unknown OS type hvm. We can't get guest capabilities. The log is as the following(log_level=1): 2003-01-02 01:21:39.895+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:21:39.896+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl 2003-01-02 01:22:13.342+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl 2003-01-02 01:22:13.347+: 12204: error : qemuCapsExtractVersion:1576 : internal error Cannot find suitable emulator for ppc64 2003-01-02 01:22:13.351+: 12205: error : qemuCapsExtractVersion:1576 : internal error Cannot find suitable emulator for ppc64 2003-01-02 01:22:13.355+: 12207: error : qemuCapsExtractVersion:1576 : internal error Cannot find suitable emulator for ppc64 2003-01-02 01:22:13.355+: 12208: error : qemuCapsExtractVersion:1576 : internal error Cannot find suitable emulator for ppc64 2003-01-02 01:22:13.357+: 12206: error : qemuCapsExtractVersion:1576 : internal error Cannot find suitable emulator for ppc64 I search online and find one similar issue from Redhat bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=509337 Is there any solution to avoid this problem? Any idea? -- Best Regards Li IBM LTC, China SystemTechnology Lab, Beijing -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt can't get capablities
Hey, On Mon, Jul 02, 2012 at 05:47:08PM +0800, Li Zhang wrote: 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl Your first problem is here, you need to have yajl-devel installed before building libvirt. Hope that helps, Christophe pgpFZWA6NUsMb.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix key error for qemuMonitorGetBlockStatsInfo
On 2012年06月21日 15:58, Osier Yang wrote: On 2012年06月21日 15:37, lvro...@linux.vnet.ibm.com wrote: From: lvroycelvro...@linux.vnet.ibm.com virDomainBlockStatsFlags can't collect total_time_ns for read/write/flush because of key typo when retriveing from qemu cmd result Signed-off-by: lvroycelvro...@linux.vnet.ibm.com --- src/qemu/qemu_monitor_json.c | 24 src/qemu/qemu_monitor_text.c | 18 +- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7d2da21..7eb9529 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1710,12 +1710,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } if (rd_total_times - virJSONValueObjectHasKey(stats, rd_total_times_ns) - (virJSONValueObjectGetNumberLong(stats, rd_total_times_ns, + virJSONValueObjectHasKey(stats, rd_total_time_ns) + (virJSONValueObjectGetNumberLong(stats, rd_total_time_ns, rd_total_times) 0)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _(cannot read %s statistic), - rd_total_times_ns); + rd_total_time_ns); goto cleanup; } if (virJSONValueObjectGetNumberLong(stats, wr_bytes, wr_bytes) 0) { @@ -1731,12 +1731,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } if (wr_total_times - virJSONValueObjectHasKey(stats, wr_total_times_ns) - (virJSONValueObjectGetNumberLong(stats, wr_total_times_ns, + virJSONValueObjectHasKey(stats, wr_total_time_ns) + (virJSONValueObjectGetNumberLong(stats, wr_total_time_ns, wr_total_times) 0)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _(cannot read %s statistic), - wr_total_times_ns); + wr_total_time_ns); goto cleanup; } if (flush_req @@ -1749,12 +1749,12 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } if (flush_total_times - virJSONValueObjectHasKey(stats, flush_total_times_ns) - (virJSONValueObjectGetNumberLong(stats, flush_total_times_ns, + virJSONValueObjectHasKey(stats, flush_total_time_ns) + (virJSONValueObjectGetNumberLong(stats, flush_total_time_ns, flush_total_times) 0)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _(cannot read %s statistic), - flush_total_times_ns); + flush_total_time_ns); goto cleanup; } } @@ -1822,12 +1822,12 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, if (STREQ(key, rd_bytes) || STREQ(key, rd_operations) || - STREQ(key, rd_total_times_ns) || + STREQ(key, rd_total_time_ns) || STREQ(key, wr_bytes) || STREQ(key, wr_operations) || - STREQ(key, wr_total_times_ns) || + STREQ(key, wr_total_time_ns) || STREQ(key, flush_operations) || - STREQ(key, flush_total_times_ns)) { + STREQ(key, flush_total_time_ns)) { num++; } else { /* wr_highest_offset is parsed by qemuMonitorJSONGetBlockExtent. */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index edb5dfd..086b06b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -977,13 +977,13 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, if (virStrToLong_ll (p,dummy, 10, wr_req) == -1) VIR_DEBUG (error reading wr_req: %s, p); } else if (rd_total_times - STRPREFIX (p, rd_total_times_ns=)) { - p += strlen(rd_total_times_ns=); + STRPREFIX (p, rd_total_time_ns=)) { + p += strlen(rd_total_time_ns=); if (virStrToLong_ll (p,dummy, 10, rd_total_times) == -1) VIR_DEBUG (error reading rd_total_times: %s, p); } else if (wr_total_times - STRPREFIX (p, wr_total_times_ns=)) { - p += strlen(wr_total_times_ns=); + STRPREFIX (p, wr_total_time_ns=)) { + p += strlen(wr_total_time_ns=); if (virStrToLong_ll (p,dummy, 10, wr_total_times) == -1) VIR_DEBUG (error reading wr_total_times: %s, p); } else if (flush_req @@ -992,8 +992,8 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, if (virStrToLong_ll (p,dummy, 10, flush_req) == -1) VIR_DEBUG (error reading flush_req: %s, p); } else if (flush_total_times - STRPREFIX (p, flush_total_times_ns=)) { - p += strlen(flush_total_times_ns=); + STRPREFIX (p, flush_total_time_ns=)) { + p += strlen(flush_total_time_ns=); if (virStrToLong_ll (p,dummy, 10, flush_total_times) == -1) VIR_DEBUG (error reading flush_total_times: %s, p); } else { @@ -1071,10 +1071,10 @@ int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, STRPREFIX (p, wr_bytes=) || STRPREFIX (p, rd_operations=) || STRPREFIX (p, wr_operations=) || - STRPREFIX (p, rd_total_times_ns=) || - STRPREFIX (p, wr_total_times_ns=) || + STRPREFIX (p, rd_total_time_ns=) || + STRPREFIX (p, wr_total_time_ns=) || STRPREFIX (p, flush_operations=) || - STRPREFIX (p, flush_total_times_ns=)) { + STRPREFIX (p, flush_total_time_ns=)) { num++; } else { VIR_DEBUG (unknown block stat near %s, p); ACK, interesting, QEMU doc has bug to fix too then. E.g. { device:sd0, stats:{ wr_highest_offset:0, wr_bytes:0, wr_operations:0, rd_bytes:0, rd_operations:0 flush_operations:0, wr_total_times_ns:0 rd_total_times_ns:0 flush_total_times_ns:0 } Hi, lvroyce. Sorry for the late, and not see
Re: [libvirt] libvirt can't get capablities
On 2012年07月02日 17:47, Li Zhang wrote: Hi, I have suffered several times on both x86 machine and power machine. When we want to use libvirt to create on VMs, it reports unknown OS type hvm. We can't get guest capabilities. The log is as the following(log_level=1): 2003-01-02 01:21:39.895+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:21:39.896+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl You need to build the pkg with yajl-devel. I'm wondering if we should force to require yajl-devel if build with with-qemu, as without yajl-devel, it can't do much work with new qemu now. Regards. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt can't get capablities
On Mon, Jul 02, 2012 at 05:47:08PM +0800, Li Zhang wrote: Hi, I have suffered several times on both x86 machine and power machine. When we want to use libvirt to create on VMs, it reports unknown OS type hvm. We can't get guest capabilities. The log is as the following(log_level=1): 2003-01-02 01:21:39.895+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:21:39.896+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl 2003-01-02 01:22:13.342+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl Hmm, this shows a flaw in the way we deal with QEMU. When detecting QEMU for the purpose of the capabilities XML, IMHO, we should *not* be raising this error. Errors in detecting capabilities are more or less invisible to users of libvirt, since they do not occur in response to a specific API call. Thus, we should have reported the capabilities XML as normal here, and only reported the YAJL compatibility error at the time the user tries to actually start a guest. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH] Add ability to managed save libvirt domains
On 06/27/2012 08:31 AM, Daniel Veillard wrote: On Sat, Jun 23, 2012 at 07:01:51AM +0400, y...@billing.fastvps.ru wrote: From: Pavel Odintsov pavel.odint...@gmail.com --- src/libvirt-php.c | 22 ++ src/libvirt-php.h |1 + 2 files changed, 23 insertions(+), 0 deletions(-) I see Michal actually commited your patch, thanks ! I'm also pushing the following to keep track of contributors :-) Adding authors of recent patches diff --git a/AUTHORS b/AUTHORS index 0999762..bfdeaa5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -19,6 +19,10 @@ There are also other people that have contributed to the proj Daniel P. Berrange berra...@redhat.com Tiziano Mueller dev-z...@gentoo.org Yukihiro Kawada warp.kaw...@gmail.com +Remi Collet r...@famillecollet.com +Ivo van den Abeelen ivovandenabee...@gmail.com +Tiziano Müller dev-z...@gentoo.org +Pavel Odintsov pavel.odint...@gmail.com If you would like to be listed in the contributors list feel free to clone this repository to your computer and Thanks a lot for this patch and commit Daniel and I apologize to anybody missing in the AUTHORS file before :-) Michal -- Michal Novotny minov...@redhat.com, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Fri, 2012-06-29 at 14:08 -0400, rmar...@linux.vnet.ibm.com wrote: From: Richa Marwaha rmar...@linux.vnet.ibm.com This patch provides AppArmor policy updates for the QEMU bridge helper. The QEMU bridge helper is a SUID executable exec'd by QEMU that drops capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge. Signed-off-by: Richa Marwaha rmar...@linux.vnet.ibm.com Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com --- examples/apparmor/libvirt-qemu | 21 - 1 files changed, 20 insertions(+), 1 deletions(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 10cdd36..766a334 100644 --- a/examples/apparmor/libvirt-qemu +++ b/examples/apparmor/libvirt-qemu @@ -1,4 +1,4 @@ -# Last Modified: Mon Apr 5 15:11:27 2010 +# Last Modified: Fri Mar 9 14:43:22 2012 #include abstractions/base #include abstractions/consoles @@ -108,3 +108,22 @@ /bin/dash rmix, /bin/dd rmix, /bin/cat rmix, + + /usr/libexec/qemu-bridge-helper Cx, + # child profile for bridge helper process + profile /usr/libexec/qemu-bridge-helper { + #include abstractions/base + + capability setuid, + capability setgid, + capability setpcap, + capability net_admin, + + network inet stream, + + /dev/net/tun rw, + /etc/qemu/** r, + owner @{PROC}/*/status r, + + /usr/libexec/qemu-bridge-helper rmix, + } Looking at the child profile itself, this seems fine. However, the Cx transition makes it so that all libvirt-managed qemu processes are allowed to execute this setuid helper and I wonder if that is too much? Ie, a guest running under libvirt's NAT wouldn't need access to this helper at all. I wonder if it would be better to adjust virt-aa-helper to add this transition and child profile instead (the child profile could theoretically still be in apparmor/libvirt-qemu, but this is a bit messy)? Can we determine from the domain XML the circumstances when /usr/libexec/qemu-bridge-helper will be used? If so, virt-aa-helper could add the access only then. As a side-benefit, handling this in virt-aa-helper allows us at compile-time to adjust the path to qemu-bridge-helper to use the configured path to the binary (ie, some distros may not use /usr/libexec). -- Jamie Strandboge | http://www.canonical.com signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Allowing promiscuous mode for domains network interfaces
Hi all, By default, OpenVZ and VirtualBox ( 4.0.x) filter network packets by MAC addresses : only broadcast, multicast and packets directly targeted to VMs are transmitted. This behaviour prevents from using promiscuous mode inside domains. I'd like to write some patches to disable these filters from libvirt. Would it be ok to modify OpenVZ and VirtualBox drivers so that they disable the filters by default ? If this is not acceptable, what about making it configurable through domains' XML ? Regards, Jean-Baptiste -- Jean-Baptiste ROUAULT Ingénieur RD - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v2] vmware: detect when a domain was shut down from the inside
On Monday 02 April 2012 15:59:32 Jean-Baptiste Rouault wrote: This patch adds an internal function vmwareUpdateVMStatus to update the real state of the domain. This function is used in various places in the driver, in particular to detect when the domain has been shut down by the user with the halt command. Hi, Bumping the thread because it seems that it was forgotten. Regards, Jean-Baptiste -- Jean-Baptiste ROUAULT Ingénieur RD - diateam : Architectes de l'information Phone : +33 (0)2 98 050 050 Fax : +33 (0)2 98 050 051 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Allowing promiscuous mode for domains network interfaces
On 07/02/2012 09:28 AM, Jean-Baptiste Rouault wrote: Hi all, By default, OpenVZ and VirtualBox ( 4.0.x) filter network packets by MAC addresses : only broadcast, multicast and packets directly targeted to VMs are transmitted. This behaviour prevents from using promiscuous mode inside domains. I'd like to write some patches to disable these filters from libvirt. Would it be ok to modify OpenVZ and VirtualBox drivers so that they disable the filters by default ? If this is not acceptable, what about making it configurable through domains' XML ? It sounds like exposing this through the domain XML would be useful to other hypervisors, and certainly something that I would rather have configurable per-guest instead of hard-coded to one default or another. We might declare that if the XML element is not present then it is up to hypervisor defaults whether the interface is promiscuous, to allow for back-compat, while still allowing the user to explicitly select narrow or promiscuous with new libvirt. -- 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] libvirt can't get capablities
On Mon, Jul 02, 2012 at 05:56:35PM +0800, Osier Yang wrote: I'm wondering if we should force to require yajl-devel if build with with-qemu, as without yajl-devel, it can't do much work with new qemu now. Something like this ? ;) commit 350583c859deaaddc98d7319e1c2ad649e4d3e83 Author: Eric Blake ebl...@redhat.com Date: Wed Jun 13 10:09:39 2012 -0600 build: hoist qemu dependence on yajl to configure Commit 6e769eba made it a runtime error if libvirt was compiled without yajl support but targets a new enough qemu. But enough users are hitting this on self-compiled libvirt that it is worth erroring out at compilation time, rather than an obscure failure when trying to use the built executable. * configure.ac: If qemu is requested and -version works, require yajl when qemu version is new enough. * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add comment. pgp185DhG8Js5.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: add rbd to whitelist of migration-safe formats
QEMU (and librbd) flush the cache on the source before the destination starts, and the destination does not read any changeable data before that, so live migration with rbd caching is safe. This makes 'virsh migrate' work with rbd and caching without the --unsafe flag. Reported-by: Vladimir Bashkirtsev vladi...@bashkirtsev.com Signed-off-by: Josh Durgin josh.dur...@inktank.com --- src/qemu/qemu_migration.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 48369d6..f51c99a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -847,6 +847,9 @@ qemuMigrationIsSafe(virDomainDefPtr def) continue; else if (cfs 0) return false; +} else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK + disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { +continue; } qemuReportError(VIR_ERR_MIGRATE_UNSAFE, %s, -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add rbd to whitelist of migration-safe formats
On 07/02/2012 12:55 PM, Josh Durgin wrote: QEMU (and librbd) flush the cache on the source before the destination starts, and the destination does not read any changeable data before that, so live migration with rbd caching is safe. This makes 'virsh migrate' work with rbd and caching without the --unsafe flag. Reported-by: Vladimir Bashkirtsev vladi...@bashkirtsev.com Signed-off-by: Josh Durgin josh.dur...@inktank.com --- src/qemu/qemu_migration.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 48369d6..f51c99a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -847,6 +847,9 @@ qemuMigrationIsSafe(virDomainDefPtr def) continue; else if (cfs 0) return false; +} else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK + disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { +continue; } ACK. However, before I push, I note that you are previously listed in AUTHORS under a different email address. Which of the two addresses do you prefer, so that we can update .mailmap and keep 'make syntax-check' happy? -- 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] qemu: add rbd to whitelist of migration-safe formats
On 07/02/2012 12:48 PM, Eric Blake wrote: On 07/02/2012 12:55 PM, Josh Durgin wrote: QEMU (and librbd) flush the cache on the source before the destination starts, and the destination does not read any changeable data before that, so live migration with rbd caching is safe. This makes 'virsh migrate' work with rbd and caching without the --unsafe flag. Reported-by: Vladimir Bashkirtsevvladi...@bashkirtsev.com Signed-off-by: Josh Durginjosh.dur...@inktank.com --- src/qemu/qemu_migration.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 48369d6..f51c99a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -847,6 +847,9 @@ qemuMigrationIsSafe(virDomainDefPtr def) continue; else if (cfs 0) return false; +} else if (disk-type == VIR_DOMAIN_DISK_TYPE_NETWORK + disk-protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { +continue; } ACK. However, before I push, I note that you are previously listed in AUTHORS under a different email address. Which of the two addresses do you prefer, so that we can update .mailmap and keep 'make syntax-check' happy? I prefer the @inktank.com one, thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] Introduce the function virCgroupForHypervisor
On 06/05/2012 02:13 AM, tangchen wrote: Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) According to your cover letter, this patch was written by Wen, but I don't see a From: listing or Signed-off-by or any other indication that would let 'git am' credit Wen as the author. Instead, it tries to credit you, using only your email alias 'tangchen' instead of your full name (again, by the cover letter, and looking at the current contents of AUTHORS, I assume you prefer 'Tang Chen'). --- src/libvirt_private.syms |1 + src/util/cgroup.c| 42 ++ src/util/cgroup.h|4 3 files changed, 47 insertions(+), 0 deletions(-) /** + * virCgroupForHypervisor: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns 0 on success or -errno value on failure. Other than that, the patch looks fine to me, but can you please rebase it to latest libvirt.git and resubmit it so that it gets recorded with correct authorship? -- 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 02/13] Introduce the function virCgroupMoveTask
On 06/05/2012 02:16 AM, tangchen wrote: Introduce a new API to move all tasks from a cgroup to another cgroup Again, authorship is incorrect for the purposes of 'git am'. --- src/libvirt_private.syms |1 + src/util/cgroup.c| 55 ++ src/util/cgroup.h|2 + 3 files changed, 58 insertions(+), 0 deletions(-) @@ -791,6 +791,61 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) return rc; } +static int virCgrouAddTaskStr(virCgroupPtr group, const char *pidstr) s/CgrouAdd/CgroupAdd/ +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) +{ +int rc = 0; +int i; +char *content, *value, *next; + +for (i = 0 ; i VIR_CGROUP_CONTROLLER_LAST ; i++) { +/* Skip over controllers not mounted */ +if (!src_group-controllers[i].mountPoint || +!dest_group-controllers[i].mountPoint) +continue; Should we insist that src_group and dest_group have the same set of mounted controllers? I'm worried that if we call this function, but the set of mounted controllers differs between the two sets, then we end up moving processes between some controllers and stranding them in the source for the remaining controllers. + +rc = virCgroupGetValueStr(src_group, i, tasks, content); +if (rc != 0) +break; Should we try to undo any changes if we fail partway through? This just breaks the outer 'for' loop and returns 0, instead of using 'goto cleanup'. + +value = content; +while((next = strchr(value, '\n')) != NULL) { Coding style: space after 'while' +*next = '\0'; +if ((rc = virCgrouAddTaskStr(dest_group, value) 0)) +goto cleanup; +value = next + 1; +} +if (*value != '\0') { +if ((rc = virCgrouAddTaskStr(dest_group, value) 0)) Does it make sense to parse all the string into integers, just to format the integers back into strings? Or would it be simpler to just cat the contents of the 'tasks' file from the source into the destination without bothering to interpret the date in transit? +goto cleanup; +} + +VIR_FREE(content); +} + +return 0; + +cleanup: +virCgroupMoveTask(dest_group, src_group); Is this cleanup always correct, or is it only correct if 'dest_group' started life empty? Should we at least log a warning message if a move was partially attempted and then reverted, particularly if the reversion attempt failed? -- 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 1/3] esx: Wrap libcurl multi handle
--- src/esx/esx_vi.c | 111 ++ src/esx/esx_vi.h | 18 + 2 files changed, 129 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 5b5ab69..48718b6 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -86,6 +86,7 @@ ESX_VI__TEMPLATE__ALLOC(CURL) ESX_VI__TEMPLATE__FREE(CURL, { esxVI_SharedCURL *shared = item-shared; +esxVI_MultiCURL *multi = item-multi; if (shared != NULL) { esxVI_SharedCURL_Remove(shared, item); @@ -95,6 +96,14 @@ ESX_VI__TEMPLATE__FREE(CURL, } } +if (multi != NULL) { +esxVI_MultiCURL_Remove(multi, item); + +if (multi-count == 0) { +esxVI_MultiCURL_Free(multi); +} +} + if (item-handle != NULL) { curl_easy_cleanup(item-handle); } @@ -555,11 +564,15 @@ esxVI_SharedCURL_Add(esxVI_SharedCURL *shared, esxVI_CURL *curl) } } +virMutexLock(curl-lock); + curl_easy_setopt(curl-handle, CURLOPT_SHARE, shared-handle); curl-shared = shared; ++shared-count; +virMutexUnlock(curl-lock); + return 0; } @@ -583,11 +596,109 @@ esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl) return -1; } +virMutexLock(curl-lock); + curl_easy_setopt(curl-handle, CURLOPT_SHARE, NULL); curl-shared = NULL; --shared-count; +virMutexUnlock(curl-lock); + +return 0; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * MultiCURL + */ + +/* esxVI_MultiCURL_Alloc */ +ESX_VI__TEMPLATE__ALLOC(MultiCURL) + +/* esxVI_MultiCURL_Free */ +ESX_VI__TEMPLATE__FREE(MultiCURL, +{ +if (item-count 0) { +/* Better leak than crash */ +VIR_ERROR(_(Trying to free MultiCURL object that is still in use)); +return; +} + +if (item-handle != NULL) { +curl_multi_cleanup(item-handle); +} +}) + +int +esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl) +{ +if (curl-handle == NULL) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot add uninitialized CURL handle to a multi handle)); +return -1; +} + +if (curl-multi != NULL) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot add CURL handle to a multi handle twice)); +return -1; +} + +if (multi-handle == NULL) { +multi-handle = curl_multi_init(); + +if (multi-handle == NULL) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Could not initialize CURL (multi))); +return -1; +} +} + +virMutexLock(curl-lock); + +curl_multi_add_handle(multi-handle, curl-handle); + +curl-multi = multi; +++multi-count; + +virMutexUnlock(curl-lock); + +return 0; +} + +int +esxVI_MultiCURL_Remove(esxVI_MultiCURL *multi, esxVI_CURL *curl) +{ +if (curl-handle == NULL) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot remove uninitialized CURL handle from a + multi handle)); +return -1; +} + +if (curl-multi == NULL) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot remove CURL handle from a multi handle when it + wasn't added before)); +return -1; +} + +if (curl-multi != multi) { +ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, _(CURL (multi) mismatch)); +return -1; +} + +virMutexLock(curl-lock); + +curl_multi_remove_handle(multi-handle, curl-handle); + +curl-multi = NULL; +--multi-count; + +virMutexUnlock(curl-lock); + return 0; } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 78d3986..9560bd2 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -85,6 +85,7 @@ typedef enum _esxVI_Occurrence esxVI_Occurrence; typedef struct _esxVI_ParsedHostCpuIdInfo esxVI_ParsedHostCpuIdInfo; typedef struct _esxVI_CURL esxVI_CURL; typedef struct _esxVI_SharedCURL esxVI_SharedCURL; +typedef struct _esxVI_MultiCURL esxVI_MultiCURL; typedef struct _esxVI_Context esxVI_Context; typedef struct _esxVI_Response esxVI_Response; typedef struct _esxVI_Enumeration esxVI_Enumeration; @@ -160,6 +161,7 @@ struct _esxVI_CURL { struct curl_slist *headers; char error[CURL_ERROR_SIZE]; esxVI_SharedCURL *shared; +esxVI_MultiCURL *multi; }; int esxVI_CURL_Alloc(esxVI_CURL **curl); @@ -188,6 +190,22 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl); /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * MultiCURL + */ + +struct _esxVI_MultiCURL { +CURLM *handle; +size_t count; +}; + +int esxVI_MultiCURL_Alloc(esxVI_MultiCURL **multi); +void esxVI_MultiCURL_Free(esxVI_MultiCURL **multi); +int esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl); +int
[libvirt] [PATCH 2/3] esx: Extend esxVI_CURL_Download for partial downloads
--- src/esx/esx_driver.c |2 +- src/esx/esx_vi.c | 27 +-- src/esx/esx_vi.h |3 ++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index db2144c..95b9286 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2802,7 +2802,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) url = virBufferContentAndReset(buffer); -if (esxVI_CURL_Download(priv-primary-curl, url, vmx) 0) { +if (esxVI_CURL_Download(priv-primary-curl, url, vmx, 0, NULL) 0) { goto cleanup; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 48718b6..3f8d745 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -355,8 +355,10 @@ esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri) } int -esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) +esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content, +unsigned long long offset, unsigned long long *length) { +char *range = NULL; virBuffer buffer = VIR_BUFFER_INITIALIZER; int responseCode = 0; @@ -365,9 +367,22 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) return -1; } +if (length != NULL *length 0) { +if (virAsprintf(range, %llu-%llu, offset, offset + *length - 1) 0) { +virReportOOMError(); +goto cleanup; +} +} else if (offset 0) { +if (virAsprintf(range, %llu-, offset) 0) { +virReportOOMError(); +goto cleanup; +} +} + virMutexLock(curl-lock); curl_easy_setopt(curl-handle, CURLOPT_URL, url); +curl_easy_setopt(curl-handle, CURLOPT_RANGE, range); curl_easy_setopt(curl-handle, CURLOPT_WRITEDATA, buffer); curl_easy_setopt(curl-handle, CURLOPT_UPLOAD, 0); curl_easy_setopt(curl-handle, CURLOPT_HTTPGET, 1); @@ -378,7 +393,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) if (responseCode 0) { goto cleanup; -} else if (responseCode != 200) { +} else if (responseCode != 200 responseCode != 206) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, _(HTTP response code %d for download from '%s'), responseCode, url); @@ -390,9 +405,15 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) goto cleanup; } +if (length != NULL) { +*length = virBufferUse(buffer); +} + *content = virBufferContentAndReset(buffer); cleanup: +VIR_FREE(range); + if (*content == NULL) { virBufferFreeAndReset(buffer); return -1; @@ -414,6 +435,7 @@ esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content) virMutexLock(curl-lock); curl_easy_setopt(curl-handle, CURLOPT_URL, url); +curl_easy_setopt(curl-handle, CURLOPT_RANGE, NULL); curl_easy_setopt(curl-handle, CURLOPT_READDATA, content); curl_easy_setopt(curl-handle, CURLOPT_UPLOAD, 1); curl_easy_setopt(curl-handle, CURLOPT_INFILESIZE, strlen(content)); @@ -1231,6 +1253,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, virMutexLock(ctx-curl-lock); curl_easy_setopt(ctx-curl-handle, CURLOPT_URL, ctx-url); +curl_easy_setopt(ctx-curl-handle, CURLOPT_RANGE, NULL); curl_easy_setopt(ctx-curl-handle, CURLOPT_WRITEDATA, buffer); curl_easy_setopt(ctx-curl-handle, CURLOPT_UPLOAD, 0); curl_easy_setopt(ctx-curl-handle, CURLOPT_POSTFIELDS, request); diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 9560bd2..49b7ca2 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -167,7 +167,8 @@ struct _esxVI_CURL { int esxVI_CURL_Alloc(esxVI_CURL **curl); void esxVI_CURL_Free(esxVI_CURL **curl); int esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri); -int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content); +int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content, +unsigned long long offset, unsigned long long *length); int esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content); -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] esx: Add volume upload and download to the storage driver
This requires new flags for the public API functions, because a VMDK volume can have metadata and content in separate files. This is not yet supported by the libvirt storage API as it currently assumes that a single file represents a volume. At least it does so in case of the volume upload and download functions. For example, the default VMDK structure created by an ESX server is a volume.vmdk file (containing metadata in plaintext key/value form) and a volume-flat.vmdk (containing the actual content of the volume). The volume.vmdk contains the name of its content file. The volume.vmdk is the one that represents the volume to the ESX server and is the one that is specified in datastore paths. The volume-flat.vmdk is basically an implementation detail here. There are also one-file VMDK volumes, as created by qemu-img when targeting the VMDK format. Such VMDK volumes starts with a 'KDMV' magic and contain metadata and content. The new METADATA and CONTENT flags allow a caller to tell the ESX storage driver which part of a volume should be up- or downloaded in case of a two-file VMDK. In case of a one-file VMDK both or no flags must be given. In case of non-VMDK volumes such as ISO or floppy images no flags must be specified. If the CONTENT flag is given the the ESX driver reads the metadata file in the datastore to figure out the name of the content file. This means that for two-file VMDKs the matadata file has to be uploaded first. The libcurl based stream driver cannot use a libcurl easy handle alone because curl_easy_perform would do the whole transfer before it returns. But there is no place in the stream handling concept that would allow for such a call to be made. The stream is driven by esxStream(Send|Recv) which is probably called multiple times to send/receive the stream in chunks. Therefore, a libcurl multi handle is used that allows to perform the data transfer in chunks and also allows to support non-blocking operations. Although, the stream callbacks for non-blocking operations are not implemented yet. --- include/libvirt/libvirt.h.in | 14 + src/Makefile.am |1 + src/esx/esx_storage_driver.c | 268 ++ src/esx/esx_stream.c | 610 ++ src/esx/esx_stream.h | 33 +++ src/esx/esx_util.c | 82 ++ src/esx/esx_util.h |2 + src/esx/esx_vi.c | 29 ++ src/esx/esx_vi.h |2 + tools/virsh.c| 22 ++- 10 files changed, 1061 insertions(+), 2 deletions(-) create mode 100644 src/esx/esx_stream.c create mode 100644 src/esx/esx_stream.h diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6e8d5dd..d11ed68 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2562,11 +2562,25 @@ virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr pool, const char *xmldesc, virStorageVolPtr clonevol, unsigned int flags); + +typedef enum { +VIR_STORAGE_VOL_DOWNLOAD_DEFAULT = 0, /* default behavior */ +VIR_STORAGE_VOL_DOWNLOAD_METADATA = 1 0, /* download metadata only */ +VIR_STORAGE_VOL_DOWNLOAD_CONTENT = 1 1, /* download content only */ +} virStorageVolDownloadFlags; + int virStorageVolDownload (virStorageVolPtr vol, virStreamPtr stream, unsigned long long offset, unsigned long long length, unsigned int flags); + +typedef enum { +VIR_STORAGE_VOL_UPLOAD_DEFAULT = 0, /* default behavior */ +VIR_STORAGE_VOL_UPLOAD_METADATA = 1 0, /* upload metadata only */ +VIR_STORAGE_VOL_UPLOAD_CONTENT = 1 1, /* upload content only */ +} virStorageVolUploadFlags; + int virStorageVolUpload (virStorageVolPtr vol, virStreamPtr stream, unsigned long long offset, diff --git a/src/Makefile.am b/src/Makefile.am index 2309984..db8fb91 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -430,6 +430,7 @@ ESX_DRIVER_SOURCES = \ esx/esx_secret_driver.c esx/esx_secret_driver.h \ esx/esx_nwfilter_driver.c esx/esx_nwfilter_driver.h \ esx/esx_util.c esx/esx_util.h \ + esx/esx_stream.c esx/esx_stream.h \ esx/esx_vi.c esx/esx_vi.h \ esx/esx_vi_methods.c
[libvirt] [PATCH 0/3] esx: Storage volume up- and download
I started the download part and the stream driver quite a while ago but stopped and put it aside as I realized that the storage volume API up- and download functions assume one file per volume. This is a problem with ESX as a VMDK can consist of two files. To solve this now I added two new flags: METADATA and CONTENT. With this flags the user can specify which part of such a volume to transfer. See patch 3/3 for more details. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
On 06/26/2012 06:54 PM, Eric Blake wrote: On 06/26/2012 04:28 PM, Corey Bryant wrote: With this proposed series, we have usage akin to: 1. pass_fd FDSET={M} - returns a string /dev/fd/N showing QEMU's view of the FD 2. drive_add file=/dev/fd/N 3. if failure: close_fd /dev/fd/N In fact, there are more steps: 4. use it successfully 5. close_fd /dev/fd/N I think it would well be possible that qemu just closes the fd when it's not used internally any more. pass-fd could have a flag indicating qemu to do that. It seems like libvirt would be in a better position to understand when a file is no longer in use, and then it can call close_fd. No? Of course the the only fd that needs to be closed is the originally passed fd. The dup'd fd's are closed by QEMU. The more we discuss this, the more I'm convinced that commands like 'block-commit' that will reopen a file to change from O_RDONLY to O_RDWR will need to have an optional argument that says the name of the file to reopen. That is, I've seen three proposals: Proposal One: enhance every command to take an fd:name protocol. PRO: Successful use of a name removes the fd from the 'getfd' list. CON: Lots of commands to change. CON: The command sequence is not atomic. CON: Block layer does not have a file name tied to the fd, so the reopen operation also needs to learn the fd:name protocol, but since we're already touching the command it's not much more work. USAGE: 1. getfd FDSET={M}, ties new fd to name 2. drive_add fd=name - looks up the fd by name from the list 3a. on success, fd is no longer in the list, drive_add consumed it 3b. on failure, libvirt must call 'closefd name' to avoid a leak 4. getfd FDSET={M2}, ties new fd to newname 5. block-commit fd=newname - looks up fd by name from the list 6a. on success, fd is no longer in the list, block-commit consumed it 6b. on failure, libvirt must call 'closefd newname' to avoid a leak Proposal Two: Create a permanent name via 'getfd' or 'pass-fd', then update that name to the new fd in advance of any operation that needs to do a reopen. PRO: All other commands work without impact by using qemu_open(), by getting a clone of the permanent name. CON: The permanent name must remain open as long as qemu might need to reopen it, and reopening needs the pass-fd force option. CON: The command sequence is not atomic. USAGE: 1. pass_fd FDSET={M} - returns an integer N (or a string /dev/fd/N) showing QEMU's permanent name of the FD 2. drive_add file=permanent name means that qemu_open() will clone the fd, and drive_add is now using yet another FD while N remains open; meanwhile, the block layer knows that the drive is tied to the permanent name 3. pass-fd force FDSET={M2} - replaces fd N with the passed M2, and still returns /dev/fd/N 4. block-commit - looks up the block-device name (/dev/fd/N), which maps back to the permanent fd, and gets a copy of the newly passed M2 5. on completion (success or failure), libvirt must call 'closefd name' to avoid a leak Proposal Three: Use thread-local fds passed alongside any command, rather than passing via a separate command PRO: All commands can now atomically handle fd passing PRO: Commands that only need a one-shot open can now use fd CON: Commands that need to reopen still need modification to allow a name override during the reopen 1. drive_add nfds=1 file=/dev/fdlist/1 FDSET={M} - on success, the fd is used as the block file, on failure it is atomically closed, so there is no leak either way. However, the block file has no permanent name. 2. block-commit nfds=1 file-override=/dev/fdlist/1 FDSET={M2} - file override must be passed again (since we have no guarantee that the /dev/fdlist/1 of _this_ command matches the command-local naming used in the previous command), but the fd is again used atomically Under proposal 3, there is no need to dup fd's, merely only to check that qemu_open(/dev/fdlist/n, flags) has compatible flags with the fd received via FDSET. And the fact that things are made atomic means that libvirt no longer has to worry about calling closefd, nor does it have to worry about being interrupted between two monitor commands and not knowing what state qemu is in. But it does mean teaching every possible command that wants to do a reopen to learn how to use fd overrides rather than to reuse the permanent name that was originally in place on the first open. Here's another option that Kevin and I discussed today on IRC. I've modified a few minor details since the discussion. And Kevin please correct me if anything is wrong. Proposal Four: Pass a set of fds via 'pass-fds'. The group of fds should all refer to the same file, but may have different access flags (ie. O_RDWR, O_RDONLY). qemu_open can then dup the fd that has the matching access mode flags. pass-fds: { 'command': 'pass-fds', 'data': {'fdsetname': 'str', '*close': 'bool'}, 'returns': 'string' } close-fds: { 'command':
Re: [libvirt] [PATCH 3/3] esx: Add volume upload and download to the storage driver
On 07/02/2012 03:44 PM, Matthias Bolte wrote: The new METADATA and CONTENT flags allow a caller to tell the ESX storage driver which part of a volume should be up- or downloaded in case of a two-file VMDK. In case of a one-file VMDK both or no flags must be given. In case of non-VMDK volumes such as ISO or floppy images no flags must be specified. These flags would also be useful for other drivers; I've complained in the past that qcow2 images under the qemu driver should be able to differentiate between metadata (let download give me the qcow2 headers) and content (return the data as seen by the guest, effectively converting from qcow2 to raw in the download). --- include/libvirt/libvirt.h.in | 14 + Before we can accept the new flags, we need to also document their effect on the functions in src/libvirt.c. I'd suggest splitting this commit into two parts - add the new flags and expose them in virsh in part 1, then implement the flags for esx in part 2. This matters in case someone else implements the flags for qemu, then wants to backport the new flags and the qemu implementation but not the esx implementation. src/Makefile.am |1 + src/esx/esx_storage_driver.c | 268 ++ src/esx/esx_stream.c | 610 ++ src/esx/esx_stream.h | 33 +++ src/esx/esx_util.c | 82 ++ src/esx/esx_util.h |2 + src/esx/esx_vi.c | 29 ++ src/esx/esx_vi.h |2 + tools/virsh.c| 22 ++- Also, needs documentation in tools/virsh.pod for the new options in virsh.c. +++ b/include/libvirt/libvirt.h.in @@ -2562,11 +2562,25 @@ virStorageVolPtrvirStorageVolCreateXMLFrom (virStoragePoolPtr pool, const char *xmldesc, virStorageVolPtr clonevol, unsigned int flags); + +typedef enum { +VIR_STORAGE_VOL_DOWNLOAD_DEFAULT = 0, /* default behavior */ I'm not sure whether it makes sense to define a name for '0'; in some cases we have done it, in others we have not. I'm not opposed to the idea, but we aren't very consistent. I do, however, like the idea of using the flags argument to support the needs of ESX in representing things through two files. I have not closely reviewed this patch for content, so much as offering this as my first impressions high-level review. + +if (STREQLEN(magic, KDMV, 4)) { +/* It's a one-file VMDK with metadata and content */ +if (flags != VIR_STORAGE_VOL_DOWNLOAD_DEFAULT +flags != (VIR_STORAGE_VOL_DOWNLOAD_METADATA | + VIR_STORAGE_VOL_DOWNLOAD_CONTENT)) { +ESX_ERROR(VIR_ERR_INVALID_ARG, %s, + _(Non or both of metadata and content flag is required for this volume)); s/Non/None/ @@ -1668,6 +1934,8 @@ static virStorageDriver esxStorageDriver = { .volLookupByPath = esxStorageVolumeLookupByPath, /* 0.8.4 */ .volCreateXML = esxStorageVolumeCreateXML, /* 0.8.4 */ .volCreateXMLFrom = esxStorageVolumeCreateXMLFrom, /* 0.8.7 */ +.volDownload = esxStorageVolumeDownload, /* 0.9.14 */ +.volUpload = esxStorageVolumeUpload, /* 0.9.14 */ I think consensus was that the next version will be numberd 0.10.0, thanks to the addition of the parallels driver. -- 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 0/3] esx: Storage volume up- and download
On Mon, Jul 2, 2012 at 4:44 PM, Matthias Bolte matthias.bo...@googlemail.com wrote: I started the download part and the stream driver quite a while ago but stopped and put it aside as I realized that the storage volume API up- and download functions assume one file per volume. This is a problem with ESX as a VMDK can consist of two files. To solve this now I added two new flags: METADATA and CONTENT. With this flags the user can specify which part of such a volume to transfer. See patch 3/3 for more details. Well technically VMDKs can have more than 2 files per disk. They have the .vmdk which is just the metadata but then the content pieces can be divided into 2G chunks. Would this be something you'd want to account for in this patch series or a future update? -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] esx: Wrap libcurl multi handle
On Mon, Jul 2, 2012 at 4:44 PM, Matthias Bolte matthias.bo...@googlemail.com wrote: --- src/esx/esx_vi.c | 111 ++ src/esx/esx_vi.h | 18 + 2 files changed, 129 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 5b5ab69..48718b6 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -86,6 +86,7 @@ ESX_VI__TEMPLATE__ALLOC(CURL) ESX_VI__TEMPLATE__FREE(CURL, { esxVI_SharedCURL *shared = item-shared; + esxVI_MultiCURL *multi = item-multi; if (shared != NULL) { esxVI_SharedCURL_Remove(shared, item); @@ -95,6 +96,14 @@ ESX_VI__TEMPLATE__FREE(CURL, } } + if (multi != NULL) { + esxVI_MultiCURL_Remove(multi, item); + + if (multi-count == 0) { + esxVI_MultiCURL_Free(multi); + } + } + if (item-handle != NULL) { curl_easy_cleanup(item-handle); } @@ -555,11 +564,15 @@ esxVI_SharedCURL_Add(esxVI_SharedCURL *shared, esxVI_CURL *curl) } } + virMutexLock(curl-lock); + curl_easy_setopt(curl-handle, CURLOPT_SHARE, shared-handle); curl-shared = shared; ++shared-count; + virMutexUnlock(curl-lock); + return 0; } @@ -583,11 +596,109 @@ esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl) return -1; } + virMutexLock(curl-lock); + curl_easy_setopt(curl-handle, CURLOPT_SHARE, NULL); curl-shared = NULL; --shared-count; + virMutexUnlock(curl-lock); + + return 0; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * MultiCURL + */ + +/* esxVI_MultiCURL_Alloc */ +ESX_VI__TEMPLATE__ALLOC(MultiCURL) + +/* esxVI_MultiCURL_Free */ +ESX_VI__TEMPLATE__FREE(MultiCURL, +{ + if (item-count 0) { + /* Better leak than crash */ + VIR_ERROR(_(Trying to free MultiCURL object that is still in use)); + return; + } + + if (item-handle != NULL) { + curl_multi_cleanup(item-handle); + } Since its a double pointer maybe setting the passed in value to NULL to prevent a double free situations? +}) + +int +esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl) +{ + if (curl-handle == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot add uninitialized CURL handle to a multi handle)); + return -1; + } + + if (curl-multi != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot add CURL handle to a multi handle twice)); + return -1; + } + + if (multi-handle == NULL) { + multi-handle = curl_multi_init(); + + if (multi-handle == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Could not initialize CURL (multi))); + return -1; + } + } + + virMutexLock(curl-lock); + + curl_multi_add_handle(multi-handle, curl-handle); + + curl-multi = multi; + ++multi-count; + + virMutexUnlock(curl-lock); + + return 0; +} + +int +esxVI_MultiCURL_Remove(esxVI_MultiCURL *multi, esxVI_CURL *curl) +{ + if (curl-handle == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot remove uninitialized CURL handle from a + multi handle)); + return -1; + } + + if (curl-multi == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, + _(Cannot remove CURL handle from a multi handle when it + wasn't added before)); + return -1; + } + + if (curl-multi != multi) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, %s, _(CURL (multi) mismatch)); + return -1; + } + + virMutexLock(curl-lock); + + curl_multi_remove_handle(multi-handle, curl-handle); + + curl-multi = NULL; + --multi-count; + + virMutexUnlock(curl-lock); Maybe add your free code here when count is 0? That way you wouldn't have to contend with a potential memory leak case when the free is called when its still ref'd. + return 0; } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 78d3986..9560bd2 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -85,6 +85,7 @@ typedef enum _esxVI_Occurrence esxVI_Occurrence; typedef struct _esxVI_ParsedHostCpuIdInfo esxVI_ParsedHostCpuIdInfo; typedef struct _esxVI_CURL esxVI_CURL; typedef struct _esxVI_SharedCURL esxVI_SharedCURL; +typedef struct _esxVI_MultiCURL esxVI_MultiCURL; typedef struct _esxVI_Context esxVI_Context; typedef struct _esxVI_Response esxVI_Response; typedef struct _esxVI_Enumeration esxVI_Enumeration; @@ -160,6 +161,7 @@ struct _esxVI_CURL { struct curl_slist *headers; char error[CURL_ERROR_SIZE]; esxVI_SharedCURL *shared; + esxVI_MultiCURL
Re: [libvirt] [Qemu-devel] [PATCH v4 0/7] file descriptor passing using pass-fd
On 07/02/2012 04:02 PM, Corey Bryant wrote: Here's another option that Kevin and I discussed today on IRC. I've modified a few minor details since the discussion. And Kevin please correct me if anything is wrong. Proposal Four: Pass a set of fds via 'pass-fds'. The group of fds should all refer to the same file, but may have different access flags (ie. O_RDWR, O_RDONLY). qemu_open can then dup the fd that has the matching access mode flags. But this means that libvirt has to open a file O_RDWR up front for any file that it _might_ need qemu to reopen later, and that qemu is now hanging on to 2 fds per fdset instead of 1 fd for the life of any client of the fdset. I see no reason why libvirt can't pass in an O_RDWR fd when qemu only needs to use an O_RDONLY fd; making qemu insist on an O_RDONLY fd makes life harder for libvirt. On the other hand, I can see from a safety standpoint that passing in an O_RDWR fd opens up more potential for errors than passing in an O_RDONLY fd, but if you don't know up front whether you will ever need to write into a file, then it would be better to pass in O_RDONLY. At least I don't see it as a security risk: passing in O_RDWR but setting SELinux permissions on the fd to only allow read() but not write() via the labeling of the fd may be possible, so that libvirt could still prevent accidental writes into an O_RDWR file that starts life only needing to service reads. pass-fds: { 'command': 'pass-fds', 'data': {'fdsetname': 'str', '*close': 'bool'}, 'returns': 'string' } This still doesn't solve Dan's complaint that things are not atomic; if the monitor dies between the pass-fds and the later use of the fdset, we would need a counterpart monitor command to query what fdsets are currently known by qemu. And like you pointed out, you didn't make it clear how a timeout mechanism would be implemented to auto-close fds that are not consumed in a fixed amount of time - would that be another optional parameter to 'pass-fds'? Or do we need a way to initially create a set with only one O_RDONLY fd, but later pass in a new O_RDWR fd but associate it with the existing set rather than creating a new set (akin to the 'force' option of 'pass-fd' in proposal two)? close-fds: { 'command': 'close-fds', 'data': {'fdsetname': 'str' } where: @fdsetname - the file descriptor set name @close - *if true QEMU closes the monitor fds when they expire. if false, fds remain on the monitor until close-fds command. PRO: Supports reopen PRO: All other commands work without impact by using qemu_open() PRO: No fd leakage if close==true specified CON: Determining when to close fds when close==true may be tricky USAGE: 1. pass-fd FDSET={M} - returns a string /dev/fdset/1) that refers to the passed set of fds. 2. drive_add file=/dev/fdset/1 -- qemu_open() uses the first fd from the list that has access flags matching the qemu_open() action flags. 3. block-commit -- qemu_open() reopens /dev/fdset/1 by using the first fd from the list that has access flags matching the qemu_open() action flags. 4. The monitor fds are closed: A) *If @close == true, qemu closes the set of fds when the timer expires B) If @close == false, libvirt must issue close-fds command to close the set of fds *How to solve this has yet to be determined. Kevin mentioned potentially using a bottom-half or a timer in qemu_close(). Still, it is certainly an option worth thinking about, and I'm hoping we can come to a solid design that everyone agrees provides the desired security and flexibility without having to recode every single existing command. -- 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] qemu: add rbd to whitelist of migration-safe formats
On 07/02/2012 02:42 PM, Josh Durgin wrote: On 07/02/2012 12:48 PM, Eric Blake wrote: On 07/02/2012 12:55 PM, Josh Durgin wrote: QEMU (and librbd) flush the cache on the source before the destination starts, and the destination does not read any changeable data before that, so live migration with rbd caching is safe. This makes 'virsh migrate' work with rbd and caching without the --unsafe flag. ACK. However, before I push, I note that you are previously listed in AUTHORS under a different email address. Which of the two addresses do you prefer, so that we can update .mailmap and keep 'make syntax-check' happy? I prefer the @inktank.com one, thanks! Done and pushed. -- 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 2/3] esx: Extend esxVI_CURL_Download for partial downloads
On Mon, Jul 2, 2012 at 4:44 PM, Matthias Bolte matthias.bo...@googlemail.com wrote: --- src/esx/esx_driver.c | 2 +- src/esx/esx_vi.c | 27 +-- src/esx/esx_vi.h | 3 ++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index db2144c..95b9286 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2802,7 +2802,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) url = virBufferContentAndReset(buffer); - if (esxVI_CURL_Download(priv-primary-curl, url, vmx) 0) { + if (esxVI_CURL_Download(priv-primary-curl, url, vmx, 0, NULL) 0) { goto cleanup; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 48718b6..3f8d745 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -355,8 +355,10 @@ esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri) } int -esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) +esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content, + unsigned long long offset, unsigned long long *length) { + char *range = NULL; virBuffer buffer = VIR_BUFFER_INITIALIZER; int responseCode = 0; @@ -365,9 +367,22 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) return -1; } + if (length != NULL *length 0) { + if (virAsprintf(range, %llu-%llu, offset, offset + *length - 1) 0) { + virReportOOMError(); + goto cleanup; + } + } else if (offset 0) { + if (virAsprintf(range, %llu-, offset) 0) { + virReportOOMError(); + goto cleanup; + } + } + virMutexLock(curl-lock); curl_easy_setopt(curl-handle, CURLOPT_URL, url); + curl_easy_setopt(curl-handle, CURLOPT_RANGE, range); curl_easy_setopt(curl-handle, CURLOPT_WRITEDATA, buffer); curl_easy_setopt(curl-handle, CURLOPT_UPLOAD, 0); curl_easy_setopt(curl-handle, CURLOPT_HTTPGET, 1); @@ -378,7 +393,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) if (responseCode 0) { goto cleanup; - } else if (responseCode != 200) { + } else if (responseCode != 200 responseCode != 206) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, _(HTTP response code %d for download from '%s'), responseCode, url); @@ -390,9 +405,15 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content) goto cleanup; } + if (length != NULL) { + *length = virBufferUse(buffer); + } + *content = virBufferContentAndReset(buffer); cleanup: + VIR_FREE(range); + if (*content == NULL) { virBufferFreeAndReset(buffer); return -1; @@ -414,6 +435,7 @@ esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content) virMutexLock(curl-lock); curl_easy_setopt(curl-handle, CURLOPT_URL, url); + curl_easy_setopt(curl-handle, CURLOPT_RANGE, NULL); curl_easy_setopt(curl-handle, CURLOPT_READDATA, content); curl_easy_setopt(curl-handle, CURLOPT_UPLOAD, 1); curl_easy_setopt(curl-handle, CURLOPT_INFILESIZE, strlen(content)); @@ -1231,6 +1253,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, virMutexLock(ctx-curl-lock); curl_easy_setopt(ctx-curl-handle, CURLOPT_URL, ctx-url); + curl_easy_setopt(ctx-curl-handle, CURLOPT_RANGE, NULL); curl_easy_setopt(ctx-curl-handle, CURLOPT_WRITEDATA, buffer); curl_easy_setopt(ctx-curl-handle, CURLOPT_UPLOAD, 0); curl_easy_setopt(ctx-curl-handle, CURLOPT_POSTFIELDS, request); diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 9560bd2..49b7ca2 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -167,7 +167,8 @@ struct _esxVI_CURL { int esxVI_CURL_Alloc(esxVI_CURL **curl); void esxVI_CURL_Free(esxVI_CURL **curl); int esxVI_CURL_Connect(esxVI_CURL *curl, esxUtil_ParsedUri *parsedUri); -int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content); +int esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char **content, + unsigned long long offset, unsigned long long *length); int esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content); -- 1.7.4.1 This patch technically allows you to operate with ranges larger than an unsigned int due to the use of unsigned long long. But virBuffer can only work with unsigned ints. So there's a very real possibility to overflow the type here when performing a download over the 4GB mark. Since the content can only be in 1 file, this is a very real possibility. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/13] create a new cgroup and move all hypervisor threads to the new cgroup
On 06/05/2012 02:16 AM, tangchen wrote: create a new cgroup and move all hypervisor threads to the new cgroup. And then we can do the other things: 1. limit only vcpu usage rather than the whole qemu 2. limit for hypervisor threads(include vhost-net threads) A really useful thing to add to this commit message would be an ascii view of the cgroup hierarchy being created. If I understand correctly, this creates the following levels: cgroup mount point libvirt subdirectory (all libvirt management) driver subdirectory (all guests tied to one driver) hypervisor subdirectory (all processes tied to one guest) vcpu subdirectory (all processes tied to one VCPU of a guest) I would almost prefer to call it a VM cgroup instead of a hypervisor cgroup (and that reflects back to naming chosen in 2/13), as I tend to think of 'hypervisor' meaning 'qemu' - the technology that drives multiple guests, while I think of 'VM' meaning 'single guest', a collection of possible multiple processes under a single qemu process umbrella for running a given guest. +int qemuSetupCgroupForHypervisor(struct qemud_driver *driver, + virDomainObjPtr vm) More evidence that the naming choice is confusing - you named the parameter 'vm' instead of 'hypervisor'. That is, I think naming this qemuSetupCgroupForVM(...) makes more sense. + +if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) { +/* If we does not know VCPU-PID mapping or all vcpu runs in the same s/vcpu runs/vcpus run/ + * thread, we cannot control each vcpu. + */ +virCgroupFree(cgroup); +return 0; It makes sense to ignore failure to set up a vcpu sub-cgroup if the user never requested the feature, with the end result being that they lose out on the functionality. But if the user explicitly requested per-vcpu usage and we can't set it up, should this return a failure? In other words, I'm worried whether we need to detect whether it is always safe to ignore the failure (as done here) or whether there are situations where setup failure should prevent running the VM until the cgroup situation is resolved. + +rc = virCgroupMoveTask(cgroup, cgroup_hypervisor); +if (rc 0) { +virReportSystemError(-rc, + _(Unable to move taks from domain cgroup to s/taks/task/, and listing the task id might be useful for diagnostic purposes. +++ b/src/qemu/qemu_process.c @@ -3674,6 +3674,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuSetupCgroupForVcpu(driver, vm) 0) goto cleanup; +VIR_DEBUG(Setting cgroup for hypervisor(if required)); s/hypervisor(if/hypervisor (if/ -- 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 04/13] Enable cpuset cgroup and synchronous vcpupin info to cgroup
On 06/05/2012 02:17 AM, tangchen wrote: This patch enables cpuset cgroup, and synchronous vcpupin info set by sched_setaffinity() to cgroup. This doesn't really give many details about what you are trying to do here. Signed-off-by: Tang Chen tangc...@cn.fujitsu.com --- src/libvirt_private.syms |2 + src/qemu/qemu_cgroup.c | 51 - src/qemu/qemu_cgroup.h |2 + src/qemu/qemu_driver.c | 43 +++--- src/util/cgroup.c| 35 ++- src/util/cgroup.h|3 ++ 6 files changed, 125 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6ff1a3b..88cc37a 100644 --- a/src/libvirt_private.syms +++ b/src/qemu/qemu_cgroup.c @@ -473,18 +473,57 @@ cleanup: rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); if (rc 0) virReportSystemError(-rc, - _(%s), - Unable to rollback cpu bandwidth period); + %s, + _(Unable to rollback cpu bandwidth period)); This hunk is an independent bug fix, and should be pushed separately. I will take care of that shortly. } return -1; } +int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainDefPtr def, + int vcpuid) +{ +int i, rc; +char *new_cpus = NULL; + +if (vcpuid 0 || vcpuid = def-vcpus) { +virReportSystemError(EINVAL, + %s: %d, _(invalid vcpuid), vcpuid); I would write this: virReportSystemError(EINVAL, _(invalid vcpuid: %d), vcpuid); +return -1; +} + +for (i = 0; i def-cputune.nvcpupin; i++) { +if (vcpuid == def-cputune.vcpupin[i]-vcpuid) { +new_cpus = virDomainCpuSetFormat(def-cputune.vcpupin[i]-cpumask, + VIR_DOMAIN_CPUMASK_LEN); +if (!new_cpus) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, +_(failed to convert cpu mask)); +goto cleanup; +} +rc = virCgroupSetCpusetCpus(cgroup, new_cpus); +if (rc 0) { +virReportSystemError(-rc, + %s, _(Unable to set cpuset.cpus)); +goto cleanup; +} +} +} +VIR_FREE(new_cpus); +return 0; + +cleanup: +if (new_cpus) +VIR_FREE(new_cpus); This fails 'make syntax-check': src/qemu/qemu_cgroup.c: if (new_cpus) VIR_FREE(new_cpus); maint.mk: found useless if before free above +return -1; +} And since you call VIR_FREE(new_cpus) on both success and failure path, I'd consolidate things. Declare this up front: int ret = -1; then the tail of the function becomes: ret = 0; cleanup: VIR_FREE(new_cpus); return ret; } @@ -556,6 +595,14 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } } +/* Set vcpupin in cgroup if vcpupin xml is provided */ +if (def-cputune.nvcpupin) { +if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { +if (qemuSetupCgroupVcpuPin(cgroup_vcpu, def, i) 0) +goto cleanup; Rather than nesting this deeply, you could use , as in: if (def-cputune.nvcpupin qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) qemuSetupCgroupVcpuPin(cgroup_vcpu, def, i) 0) goto cleanup; @@ -3605,9 +3607,37 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, if (flags VIR_DOMAIN_AFFECT_LIVE) { if (priv-vcpupids != NULL) { +/* Add config to vm-def first, because cgroup APIs need it. */ +if (virDomainVcpuPinAdd(vm-def, cpumap, maplen, vcpu) 0) { +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(failed to update or add vcpupin xml of + a running domain)); +goto cleanup; +} + +/* Configure the corresponding cpuset cgroup before set affinity. */ +if (qemuCgroupControllerActive(driver, + VIR_CGROUP_CONTROLLER_CPUSET)) { +if (virCgroupForDomain(driver-cgroup, vm-def-name, + cgroup_dom, 0) == 0) { +if (virCgroupForVcpu(cgroup_dom, vcpu, cgroup_vcpu, 0) == 0) { +if (qemuSetupCgroupVcpuPin(cgroup_vcpu, vm-def, vcpu) 0) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s %d, +_(failed to set cpuset.cpus in cgroup +
Re: [libvirt] [PATCH] qemu_agent: support guest-info command
Mr. Daniel. I rewrote the patch for libvirt-0.9.13 to move some codes to libvirt-qemu as you suggested. But I am sorry for unusing git, because my joining company is very elegant for network security. So, I have an only way via http to get libvirt's source code... Regards MATSUDA Daiki virsh # help QEMU Guest Agent (help keyword 'guestagent'): qemu-agent-command Qemu Guest Agent Command Virsh itself (help keyword 'virsh'): cd change the current directory echo echo arguments exit quit this interactive terminal help print help pwdprint the current directory quit quit this interactive terminal virsh # help qemu-agent-command NAME qemu-agent-command - Qemu Guest Agent Command SYNOPSIS qemu-agent-command domain {[--cmd] string}... DESCRIPTION Qemu Guest Agent Command OPTIONS [--domain] string domain name, id or uuid [--cmd] string command virsh # qemu-agent-command RHEL58_64 guest-info {return:{version:1.1.50,supported_commands:[{enabled:true,name:guest-network-get-interfaces},{enabled:true,name:guest-suspend-hybrid},{enabled:true,name:guest-suspend-ram},{enabled:true,name:guest-suspend-disk},{enabled:true,name:guest-fsfreeze-thaw},{enabled:true,name:guest-fsfreeze-freeze},{enabled:true,name:guest-fsfreeze-status},{enabled:true,name:guest-file-flush},{enabled:true,name:guest-file-seek},{enabled:true,name:guest-file-write},{enabled:true,name:guest-file-read},{enabled:true,name:guest-file-close},{enabled:true,name:guest-file-open},{enabled:true,name:guest-shutdown},{enabled:true,name:guest-info},{enabled:true,name:guest-ping},{enabled:true,name:guest-sync},{enabled:true,name:guest-sync-delimited}]}} On Fri, Jun 29, 2012 at 01:58:05PM +0900, MATSUDA, Daiki wrote: diff -uNrp libvirt-0.9.13.orig/daemon/remote.c libvirt-0.9.13/daemon/remote.c --- libvirt-0.9.13.orig/daemon/remote.c 2012-06-25 16:06:18.0 +0900 +++ libvirt-0.9.13/daemon/remote.c 2012-06-29 12:50:03.752806682 +0900 @@ -3923,6 +3923,42 @@ cleanup: return rv; } + +static int +remoteDispatchDomainQemuAgentCommand(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_qemu_agent_command_args *args, + remote_domain_qemu_agent_command_ret *ret) +{ +virDomainPtr dom = NULL; +int rv = -1; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv-conn) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open)); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv-conn, args-dom))) +goto cleanup; + +if (virDomainQemuAgentCommand(dom, args-cmd, ret-result, args-flags) 0) { +virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(Guest Agent Error)); +goto cleanup; +} + +rv = 0; +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + /*- Helpers. -*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff -uNrp libvirt-0.9.13.orig/daemon/remote_dispatch.h libvirt-0.9.13/daemon/remote_dispatch.h --- libvirt-0.9.13.orig/daemon/remote_dispatch.h2012-06-25 19:48:08.0 +0900 +++ libvirt-0.9.13/daemon/remote_dispatch.h 2012-06-29 10:21:21.460454579 +0900 @@ -12889,6 +12889,28 @@ static int remoteDispatchSupportsFeature +static int remoteDispatchDomainQemuAgentCommand( +virNetServerPtr server, +virNetServerClientPtr client, +virNetMessagePtr msg, +virNetMessageErrorPtr rerr, +remote_domain_qemu_agent_command_args *args, +remote_domain_qemu_agent_command_ret *ret); +static int remoteDispatchDomainQemuAgentCommandHelper( +virNetServerPtr server, +virNetServerClientPtr client, +virNetMessagePtr msg, +virNetMessageErrorPtr rerr, +void *args, +void *ret) +{ + VIR_DEBUG(server=%p client=%p msg=%p rerr=%p args=%p ret=%p, server, client, msg, rerr, args, ret); + return remoteDispatchDomainQemuAgentCommand(server, client, msg, rerr, args, ret); +} +/* remoteDispatchDomainQemuAgentCommand body has to be implemented manually */ + + + virNetServerProgramProc remoteProcs[] = { { /* Unused 0 */ NULL, @@ -15374,5 +15396,14 @@ virNetServerProgramProc remoteProcs[] = true, 1 }, +{ /* Method DomainQemuAgentCommand = 276 */ + remoteDispatchDomainQemuAgentCommandHelper, + sizeof(remote_domain_qemu_agent_command_args), + (xdrproc_t)xdr_remote_qemu_agent_command_args, +
Re: [libvirt] libvirt can't get capablities
On 07/02/2012 05:53 PM, Christophe Fergeau wrote: Hey, On Mon, Jul 02, 2012 at 05:47:08PM +0800, Li Zhang wrote: 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl Your first problem is here, you need to have yajl-devel installed before building libvirt. Hi, I tried on my x86 machine to installed all these packages. [root@mcd ~]# rpm -qa yajl yajl-1.0.12-1.fc16.x86_64 [root@mcd ~]# rpm -qa yajl-devel yajl-devel-1.0.12-1.fc16.x86_64 [root@mcd ~]# lsmod |grep kvm kvm_intel 119203 0 kvm 347474 1 kvm_intel [root@mcd ~]# virsh capabilities capabilities host uuidd4ff0166-0f8b-e111-ab17-78a277034000/uuid cpu archx86_64/arch modelNehalem/model vendorIntel/vendor topology sockets='1' cores='4' threads='1'/ feature name='rdtscp'/ feature name='avx'/ feature name='osxsave'/ feature name='xsave'/ feature name='tsc-deadline'/ feature name='x2apic'/ feature name='pdcm'/ feature name='xtpr'/ feature name='tm2'/ feature name='est'/ feature name='smx'/ feature name='vmx'/ feature name='ds_cpl'/ feature name='monitor'/ feature name='dtes64'/ feature name='pclmuldq'/ feature name='pbe'/ feature name='tm'/ feature name='ht'/ feature name='ss'/ feature name='acpi'/ feature name='ds'/ feature name='vme'/ /cpu power_management suspend_mem/ suspend_disk/ suspend_hybrid/ /power_management migration_features live/ uri_transports uri_transporttcp/uri_transport /uri_transports /migration_features secmodel modelselinux/model doi0/doi /secmodel /host /capabilities Hope that helps, Christophe -- Best Regards Li IBM LTC, China SystemTechnology Lab, Beijing -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt can't get capablities
On 07/02/2012 05:56 PM, Osier Yang wrote: On 2012年07月02日 17:47, Li Zhang wrote: Hi, I have suffered several times on both x86 machine and power machine. When we want to use libvirt to create on VMs, it reports unknown OS type hvm. We can't get guest capabilities. The log is as the following(log_level=1): 2003-01-02 01:21:39.895+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:21:39.896+: 12214: error : virDomainDefParseXML:8491 : unknown OS type hvm 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl You need to build the pkg with yajl-devel. Hi Osier, I tried, but it seems that it doesn't work. Will yajl packages help to detect kvm module and qemu binary? It seems that it can't find kvm module and qemu binary. I'm wondering if we should force to require yajl-devel if build with with-qemu, as without yajl-devel, it can't do much work with new qemu now. Regards. Osier -- Best Regards Li IBM LTC, China SystemTechnology Lab, Beijing -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/13] Introduce the function virCgroupForHypervisor
Hi~ On 07/03/2012 04:40 AM, Eric Blake wrote: On 06/05/2012 02:13 AM, tangchen wrote: Introduce the function virCgroupForHypervisor() to create sub directory for hypervisor thread(include I/O thread, vhost-net thread) According to your cover letter, this patch was written by Wen, but I don't see a From: listing or Signed-off-by or any other indication that would let 'git am' credit Wen as the author. Instead, it tries to credit you, using only your email alias 'tangchen' instead of your full name (again, by the cover letter, and looking at the current contents of AUTHORS, I assume you prefer 'Tang Chen'). I forgot to change my git config when making the patches. Sorry about that. Thanks, I will fix it.:) And the other comments, I am working on it. :) --- src/libvirt_private.syms |1 + src/util/cgroup.c| 42 ++ src/util/cgroup.h|4 3 files changed, 47 insertions(+), 0 deletions(-) /** + * virCgroupForHypervisor: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns 0 on success or -errno value on failure. Other than that, the patch looks fine to me, but can you please rebase it to latest libvirt.git and resubmit it so that it gets recorded with correct authorship? -- Best Regards, Tang chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt can't get capablities
On 07/03/2012 09:12 AM, Li Zhang wrote: On 07/02/2012 05:53 PM, Christophe Fergeau wrote: Hey, On Mon, Jul 02, 2012 at 05:47:08PM +0800, Li Zhang wrote: 2003-01-02 01:22:00.135+: 12207: error : qemuCapsComputeCmdFlags:1218 : unsupported configuration: this qemu binary requires libvirt to be compiled with yajl Your first problem is here, you need to have yajl-devel installed before building libvirt. Hi, I tried on my x86 machine to installed all these packages. [root@mcd ~]# rpm -qa yajl yajl-1.0.12-1.fc16.x86_64 [root@mcd ~]# rpm -qa yajl-devel yajl-devel-1.0.12-1.fc16.x86_64 [root@mcd ~]# lsmod |grep kvm kvm_intel 119203 0 kvm 347474 1 kvm_intel [root@mcd ~]# virsh capabilities capabilities host uuidd4ff0166-0f8b-e111-ab17-78a277034000/uuid cpu archx86_64/arch modelNehalem/model vendorIntel/vendor topology sockets='1' cores='4' threads='1'/ feature name='rdtscp'/ feature name='avx'/ feature name='osxsave'/ feature name='xsave'/ feature name='tsc-deadline'/ feature name='x2apic'/ feature name='pdcm'/ feature name='xtpr'/ feature name='tm2'/ feature name='est'/ feature name='smx'/ feature name='vmx'/ feature name='ds_cpl'/ feature name='monitor'/ feature name='dtes64'/ feature name='pclmuldq'/ feature name='pbe'/ feature name='tm'/ feature name='ht'/ feature name='ss'/ feature name='acpi'/ feature name='ds'/ feature name='vme'/ /cpu power_management suspend_mem/ suspend_disk/ suspend_hybrid/ /power_management migration_features live/ uri_transports uri_transporttcp/uri_transport /uri_transports /migration_features secmodel modelselinux/model doi0/doi /secmodel /host /capabilities It may be caused some packages which are required. I am not quite sure about that. I reinstall all the packages required. As the following: Libtool (glibtoolize toolize) Gnutls Gnutls-devel Gnutls-c++ libgnutls pkgconfig(libtasn1) libgpg-error-devel libgcrypt-devel libtasn1-devel device-mapper-devel/libdevmapper = 1.0.0 Python-devel Gettext-devel (autopoint) Htmldoc Libxml2-devel xhtml1-dtds And remove libvirt source code, then git clone latest source code. Then I compile it and install libvirt. It seems that it works. Hope that helps, Christophe -- Best Regards Li IBM LTC, China SystemTechnology Lab, Beijing -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list