Re: [libvirt] [PATCH 12/33] Fix error reporting in port profile parsing/formatting APIs
After applying this patch, make fails with: CC libvirt_util_la-network.lo cc1: warnings being treated as errors util/network.c: In function 'virNetDevVPortProfileParse': util/network.c:712:23: error: assignment makes pointer from integer without a cast util/network.c:712:69: error: ordered comparison of pointer with integer zero [-Wextra] On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com The virtual port profile parsing/formatting APIs do not correctly handle unknown profile type strings/numbers. They behave as a no-op, instead of raising an error Actually I've noticed a *lot* of the *Format functions ignore the possibility of bad values in the vir*Def objects (e.g. invalid enum values), and have debated with myself about whether to ignore or report invalid data. * src/util/network.c, src/util/network.h: Fix error handling of port profile APIs * src/conf/domain_conf.c, src/conf/network_conf.c: Update for API changes --- src/conf/domain_conf.c | 16 +++- src/conf/network_conf.c | 20 +++--- src/util/network.c | 61 --- src/util/network.h |8 +++--- 4 files changed, 54 insertions(+), 51 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 023eb9f..5e38bee 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -700,13 +699,19 @@ virNetDevVPortProfileParse(xmlNodePtr node, if (VIR_ALLOC(virtPort) 0) { virReportOOMError(); -return -1; +return NULL; } virtPortType = virXMLPropString(node, type); if (!virtPortType) { virSocketError(VIR_ERR_XML_ERROR, %s, - _(missing virtualportprofile type)); + _(missing virtualportprofile type)); +goto error; +} The following should be virtPort-virPortType = : + +if ((virtPortType = virNetDevVPortTypeFromString(virtPortType))= 0) { +virSocketError(VIR_ERR_XML_ERROR, + _(unknown virtualportprofile type %s), virtPortType); goto error; } @@ -879,23 +877,20 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr return true; } -void + +int virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, virBufferPtr buf) { char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!virtPort || virtPort-virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE) -return; +return 0; virBufferAsprintf(buf, virtualport type='%s'\n, virNetDevVPortTypeToString(virtPort-virtPortType)); switch (virtPort-virtPortType) { -case VIR_NETDEV_VPORT_PROFILE_NONE: -case VIR_NETDEV_VPORT_PROFILE_LAST: -break; - case VIR_NETDEV_VPORT_PROFILE_8021QBG: virUUIDFormat(virtPort-u.virtPort8021Qbg.instanceID, uuidstr); @@ -913,9 +908,15 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, parameters profileid='%s'/\n, virtPort-u.virtPort8021Qbh.profileID); break; + +default: +virSocketError(VIR_ERR_XML_ERROR, + _(unexpected virtualport type %d), virtPort-virtPortType); +return -1; } A bit of digression: In the example here, you're doing something with the enum value aside from just converting it to a string, so there's a ready place to put in the check and return failure if it's out of bounds/unknown, but there are many many places where an XXXTypeToString() macro is used directly as an argument in a printf with no check for validity. On one hand, this is a bit sloppy if we consider that the [whatever]Def objects may contain unverified data (downright dangerous if you think about platforms that don't protect against NULL dereferences in printf!); on the other hand, if we changed every occurrence of that to get the string value into a temp and check for non-NULL before using it in a printf, it would add significant clutter to the code (domain_conf.c seems to be the biggest offender here). Even in this case, we're calling virBufferAsprintf with the unqualified return from virNetDevVPortTypeToString before we eventually vet it in the following switch statement. So do we consider objects sent to the Format functions to contain qualified data or not? If not, there's quite a large patch waiting in the wings :-) Anyway, ACK with the compile problem fixed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/14] Add support for systemd init service
Hi Daniel, On Thu, Jul 07, 2011 at 03:17:24PM +0100, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com This patch adds support for a systemd init service for libvirtd and libvirt-guests. The libvirtd.service is *not* written to use socket activation, since we want libvirtd to start on boot so it can do guest auto-start. It seems this nevert got pushed. I't be great to have systemd support upstream so we don't reinvent the wheel on every distro that uses systemd. Cheers, -- Guido The libvirt-guests.service is pretty lame, just exec'ing the original init script for now. Ideally we would factor out the functionality, into some shared tool. Instead of ./configure --with-init-script=redhat You can now do ./configure --with-init-script=systemd Or better still: ./configure --with-init-script=systemd+redhat * configure.ac: Add systemd, and systemd+redhat options to --with-init-script option * daemon/Makefile.am: Install systemd services * daemon/libvirtd.sysconf: Add note about unused env variable with systemd * daemon/libvirtd.service.in: libvirtd systemd service unit * libvirt.spec.in: Add scripts to installing systemd services and migrating from legacy init scripts * tools/Makefile.am: Install systemd services * tools/libvirt-guests.init.sh: Rename to tools/libvirt-guests.init.in * tools/libvirt-guests.service.in: systemd service unit --- configure.ac | 32 +-- daemon/.gitignore |1 + daemon/Makefile.am | 62 ++ daemon/libvirtd.service.in | 20 daemon/libvirtd.sysconf|3 + libvirt.spec.in| 92 +++- tools/Makefile.am | 63 +++--- ...bvirt-guests.init.sh = libvirt-guests.init.in} |0 tools/libvirt-guests.service.in| 13 +++ 9 files changed, 245 insertions(+), 41 deletions(-) create mode 100644 daemon/libvirtd.service.in rename tools/{libvirt-guests.init.sh = libvirt-guests.init.in} (100%) create mode 100644 tools/libvirt-guests.service.in diff --git a/configure.ac b/configure.ac index 13b4afb..d70e574 100644 --- a/configure.ac +++ b/configure.ac @@ -305,16 +305,30 @@ dnl init script flavor dnl AC_MSG_CHECKING([for init script flavor]) AC_ARG_WITH([init-script], -[AC_HELP_STRING([--with-init-script=@:@redhat|auto|none@:@], + [AC_HELP_STRING([--with-init-script=@:@redhat|systemd|systemd+redhat|upstart|auto|none@:@], [Style of init script to install @:@default=auto@:@])]) -if test x$with_init_script = x || test x$with_init_script = xauto; then -if test $cross_compiling = yes || test ! -f /etc/redhat-release; then -with_init_script=none -else -with_init_script=redhat -fi -fi -AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test x$with_init_script = xredhat) +init_redhat=no +init_systemd=no +case $with_init_script in +systemd+redhat) + init_redhat=yes + init_systemd=yes + ;; +systemd) + init_systemd=yes + ;; +redhat) + init_redhat=yes + ;; +*) + if test $cross_compiling != yes test -f /etc/redhat-release; then + init_redhat=yes + with_init_script=redhat + fi + ;; +esac +AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test $init_redhat = yes) +AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_SYSTEMD], test $init_systemd = yes) AC_MSG_RESULT($with_init_script) dnl RHEL-5 has a peculiar version of Xen, which requires some special casing diff --git a/daemon/.gitignore b/daemon/.gitignore index ab3d093..2873143 100644 --- a/daemon/.gitignore +++ b/daemon/.gitignore @@ -7,6 +7,7 @@ Makefile.in libvirt_qemud libvirtd libvirtd.init +libvirtd.service libvirtd*.logrotate libvirtd.8 libvirtd.8.in diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 8ed29b8..abb5e1f 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -180,13 +180,13 @@ probes.o: probes.d CLEANFILES += probes.h probes.o endif -install-data-local: install-init install-data-sasl install-data-polkit \ +install-data-local: install-init install-systemd install-data-sasl install-data-polkit \ install-logrotate mkdir -p $(DESTDIR)$(localstatedir)/log/libvirt mkdir -p $(DESTDIR)$(localstatedir)/run/libvirt mkdir -p $(DESTDIR)$(localstatedir)/lib/libvirt -uninstall-local:: uninstall-init uninstall-data-sasl uninstall-data-polkit +uninstall-local:: uninstall-init uninstall-systemd uninstall-data-sasl uninstall-data-polkit rmdir $(DESTDIR)$(localstatedir)/log/libvirt || : rmdir $(DESTDIR)$(localstatedir)/run/libvirt || : rmdir
Re: [libvirt] nwfilter - limit VM traffic to specific mac address
On 08.11.11 16:34, Stefan Berger wrote: On 11/07/2011 04:25 AM, Shahar Havivi wrote: Hi, I want to limit VM traffic to a specific MAC address, ie VMs cannot traffic each other other then a specific gateway. I am using custom nwfilter name: isolatedprivatevlan-vdsm.xml located in /etc/libvirt/nwfilter/: filter name='isolatedprivatevlan-vdsm' chain='root' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='500' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Try this one -- it works in 'my' subnet: filter name='isolatedprivatevlan-vdsm' chain='ipv4' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='10' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Thanks, Now it is blocking the traffic but I can't get traffic to the gateway as well... VM1 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:53/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface VM2 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:52/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface in each VM (Fedora 15 LiveCD) I assign ip: # ifconfig eth0 10.35.1.240 netmask 255.255.254.0 # route add default gw 10.35.1.1 vm2: # ifconfig eth0 10.35.1.241 netmask 255.255.254.0 # route add default gw 10.35.1.1 but the filter is not working, I can ping the VMs from each other, Am I missing something? Try the above filter that puts the check into a different 'chain' into different order. I'll be introducing a 'mac' chain where this can then be put into rather than into the 'ipv4' chain. The challenging part about the filtering rules is their order and the XML can unfortunately not abstract this 'away'. Stefan Thanks, Shahar Havivi. -- 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 13/33] Split src/util/network.{c, h} into 5 pieces
(Of course fixing the two problems I noted in earlier patches 1) caused a conflict when applying this patch and 2) caused a new build failure with make check ) ACK on this, aside from some files wrongly attributed to me :-) On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com The src/util/network.c file is a dumping ground for many different APIs. Split it up into 5 pieces, along functional lines - src/util/virnetdevbandwidth.c: virNetDevBandwidth type helper APIs - src/util/virnetdevvportprofile.c: virNetDevVPortProfile type helper APIs - src/util/virsocketaddr.c: virSocketAddr and APIs - src/conf/netdev_bandwidth_conf.c: XML parsing / formatting for virNetDevBandwidth - src/conf/netdev_vport_profile_conf.c: XML parsing / formatting for virNetDevVPortProfile * src/util/network.c, src/util/network.h: Split into 5 pieces * src/conf/netdev_bandwidth_conf.c, src/conf/netdev_bandwidth_conf.h, src/conf/netdev_vport_profile_conf.c, src/conf/netdev_vport_profile_conf.h, src/util/virnetdevbandwidth.c, src/util/virnetdevbandwidth.h, src/util/virnetdevvportprofile.c, src/util/virnetdevvportprofile.h, src/util/virsocketaddr.c, src/util/virsocketaddr.h: New pieces * daemon/libvirtd.h, daemon/remote.c, src/conf/domain_conf.c, src/conf/domain_conf.h, src/conf/network_conf.c, src/conf/network_conf.h, src/conf/nwfilter_conf.h, src/esx/esx_util.h, src/network/bridge_driver.c, src/qemu/qemu_conf.c, src/rpc/virnetsocket.c, src/rpc/virnetsocket.h, src/util/dnsmasq.h, src/util/interface.h, src/util/iptables.h, src/util/macvtap.c, src/util/macvtap.h, src/util/virnetdev.h, src/util/virnetdevtap.c, tools/virsh.c: Update include files --- daemon/libvirtd.h|1 - daemon/remote.c |1 - po/POTFILES.in |4 +- src/Makefile.am | 12 +- src/conf/domain_conf.c |3 +- src/conf/domain_conf.h |4 +- src/conf/netdev_bandwidth_conf.c | 230 ++ src/conf/netdev_bandwidth_conf.h | 37 + src/conf/netdev_vport_profile_conf.c | 236 ++ src/conf/netdev_vport_profile_conf.h | 39 + src/conf/network_conf.c |3 +- src/conf/network_conf.h |4 +- src/conf/nwfilter_conf.h |2 +- src/esx/esx_util.h |2 +- src/network/bridge_driver.c |1 - src/qemu/qemu_conf.c |1 - src/rpc/virnetsocket.c |1 + src/rpc/virnetsocket.h |2 +- src/util/dnsmasq.h |2 +- src/util/interface.h |2 +- src/util/iptables.h |2 +- src/util/macvtap.c |1 - src/util/macvtap.h |8 +- src/util/network.c | 1349 -- src/util/network.h | 173 - src/util/virnetdev.h |2 +- src/util/virnetdevbandwidth.c| 265 +++ src/util/virnetdevbandwidth.h| 53 ++ src/util/virnetdevtap.c |1 + src/util/virnetdevvportprofile.c | 62 ++ src/util/virnetdevvportprofile.h | 64 ++ src/util/virsocketaddr.c | 687 + src/util/virsocketaddr.h | 103 +++ tools/virsh.c|2 +- 34 files changed, 1813 insertions(+), 1546 deletions(-) create mode 100644 src/conf/netdev_bandwidth_conf.c create mode 100644 src/conf/netdev_bandwidth_conf.h create mode 100644 src/conf/netdev_vport_profile_conf.c create mode 100644 src/conf/netdev_vport_profile_conf.h delete mode 100644 src/util/network.c delete mode 100644 src/util/network.h create mode 100644 src/util/virnetdevbandwidth.c create mode 100644 src/util/virnetdevbandwidth.h create mode 100644 src/util/virnetdevvportprofile.c create mode 100644 src/util/virnetdevvportprofile.h create mode 100644 src/util/virsocketaddr.c create mode 100644 src/util/virsocketaddr.h diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c new file mode 100644 index 000..63c6668 --- /dev/null +++ b/src/conf/netdev_vport_profile_conf.c @@ -0,0 +1,236 @@ +/* + * Copyright (C) 2009-2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + *
Re: [libvirt] [PATCH] lxc: plug memory leak
On 09.11.2011 07:20, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected by Coverity. Leak introduced in commit 9d201a5. * src/lxc/lxc_driver.c: Clean up on failure. Signed-off-by: Alex Jia a...@redhat.com --- src/lxc/lxc_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 37092bc..295fd6f 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1856,7 +1856,7 @@ cleanup: if (err) { virSetError(err); -virResetError(err); +virFreeError(err); } return rc; ACKed and pushed. However, I've changed the title to: lxc: free error object to avoid memory leak -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: plug memory leak
On 09.11.2011 07:51, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected by Coverity. Leak introduced in commit 0f31f7b. * src/lxc/lxc_driver.c: Clean up on failure. Signed-off-by: Alex Jia a...@redhat.com --- src/lxc/lxc_driver.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 37092bc..a1d0a7c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1850,6 +1850,7 @@ cleanup: } for (i = 0 ; i nttyFDs ; i++) VIR_FORCE_CLOSE(ttyFDs[i]); +VIR_FREE(ttyFDs); VIR_FORCE_CLOSE(handshakefds[0]); VIR_FORCE_CLOSE(handshakefds[1]); VIR_FREE(logfile); ACKed and pushed I've changed the title to: lxc: free 'ttyFDs' array on return from lxcVmStart as you've sent 2 patches with same title, and I think new title express better what has changed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: plug memory leak
Michal, thanks a lot:) Regards, Alex - Original Message - From: Michal Privoznik mpriv...@redhat.com To: a...@redhat.com Cc: libvir-list@redhat.com Sent: Wednesday, November 9, 2011 5:33:22 PM Subject: Re: [libvirt] [PATCH] lxc: plug memory leak On 09.11.2011 07:51, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected by Coverity. Leak introduced in commit 0f31f7b. * src/lxc/lxc_driver.c: Clean up on failure. Signed-off-by: Alex Jia a...@redhat.com --- src/lxc/lxc_driver.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 37092bc..a1d0a7c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1850,6 +1850,7 @@ cleanup: } for (i = 0 ; i nttyFDs ; i++) VIR_FORCE_CLOSE(ttyFDs[i]); +VIR_FREE(ttyFDs); VIR_FORCE_CLOSE(handshakefds[0]); VIR_FORCE_CLOSE(handshakefds[1]); VIR_FREE(logfile); ACKed and pushed I've changed the title to: lxc: free 'ttyFDs' array on return from lxcVmStart as you've sent 2 patches with same title, and I think new title express better what has changed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/33] Rename the LXC veth management APIs and delete duplicated APIs
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com The src/lxc/veth.c file contains APIs for managing veth devices, but some of the APIs duplicate stuff from src/util/virnetdev.h. Delete thed duplicate APIs and rename the remaining ones to follow virNetDevVeth * src/lxc/veth.c, src/lxc/veth.h: Rename APIs delete duplicates * src/lxc/lxc_container.c, src/lxc/lxc_controller.c, src/lxc/lxc_driver.c: Update for API renaming --- po/POTFILES.in |1 - src/lxc/lxc_container.c |7 ++- src/lxc/lxc_controller.c |4 +- src/lxc/lxc_driver.c | 19 +++- src/lxc/veth.c | 109 - src/lxc/veth.h | 20 +++- 6 files changed, 42 insertions(+), 118 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/33] Move LXC veth.c code into shared utility APIs
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Move the virNetDevSetName and virNetDevSetNamespace APIs out of LXC's veth.c and into virnetdev.c. Move the remaining content of the file to src/util/virnetdevveth.c * src/lxc/veth.c: Rename to src/util/virnetdevveth.c * src/lxc/veth.h: Rename to src/util/virnetdevveth.h * src/util/virnetdev.c, src/util/virnetdev.h: Add virNetDevSetName and virNetDevSetNamespace * src/lxc/lxc_container.c, src/lxc/lxc_controller.c, src/lxc/lxc_driver.c: Update include paths --- src/Makefile.am |7 +- src/lxc/lxc_container.c |2 +- src/lxc/lxc_controller.c |3 +- src/lxc/lxc_driver.c |2 +- src/lxc/veth.h | 29 - src/util/virnetdev.c | 81 + src/util/virnetdev.h |4 + src/{lxc/veth.c = util/virnetdevveth.c} | 95 + src/util/virnetdevveth.h | 35 +++ 9 files changed, 144 insertions(+), 114 deletions(-) delete mode 100644 src/lxc/veth.h rename src/{lxc/veth.c = util/virnetdevveth.c} (74%) create mode 100644 src/util/virnetdevveth.h ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7
On 09.11.2011 07:46, Justin Clift wrote: Hi guys, Just checking 0.9.7 on RHEL 6.1 x86_64. Noticed this when compiling with make -j 3: make[3]: Entering directory `/home/jc/git_repos/libvirt/src' CC libvirt_util_la-bitmap.lo CC libvirt_util_la-authhelper.lo CC libvirt_util_la-bridge.lo CC libvirt_util_la-buf.lo CC libvirt_util_la-command.lo util/buf.c: In function 'virBufferEscape': util/buf.c:469: warning: logical '' with non-zero constant will always evaluate as true [-Wlogical-op] ... I've ran gcc -E on util/buf.c and interesting things are happening there: the original code: while (*cur != 0) { if (strchr(toescape, *cur)) *out++ = '\\'; *out++ = *cur; cur++; } [line 469 is: if (strchr(...))] after preprocessing stage: while (*cur != 0) { if ((__extension__ (__builtin_constant_p (*cur) !__builtin_constant_p (toescape) (*cur) == '\0' ? (char *) __rawmemchr (toescape, *cur) : __builtin_strchr (toescape, *cur *out++ = '\\'; *out++ = *cur; cur++; } So I think the problem lies within !__builtin_constant_p (toescape) but i am not sure how to fix that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND v5] Export KVM Host Power Management capabilities
This patch exports KVM Host Power Management capabilities as XML so that higher-level systems management software can make use of these features available in the host. The script pm-is-supported (from pm-utils package) is run to discover if Suspend-to-RAM (S3) or Suspend-to-Disk (S4) is supported by the host. If either of them are supported, then a new tag power_management is introduced in the XML under the host tag. Eg: When the host supports both S3 and S4, the XML looks like this: capabilities host uuiddc699581-48a2-11cb-b8a8-9a0265a79bbe/uuid cpu archi686/arch modelcoreduo/model vendorIntel/vendor topology sockets='1' cores='2' threads='1'/ feature name='xtpr'/ feature name='tm2'/ feature name='est'/ feature name='vmx'/ feature name='pbe'/ feature name='tm'/ feature name='ht'/ feature name='ss'/ feature name='acpi'/ feature name='ds'/ /cpu power_management === New host power management features S3/ S4/ /power_management migration_features live/ uri_transports uri_transporttcp/uri_transport /uri_transports /migration_features /host . . . However in case the query to check for power management features succeeded, but the host does not support any such feature, then the XML will contain an empty power_management/ tag. In the event that the PM query itself failed, the XML will not contain any power_management tag. To use this, new APIs could be implemented in libvirt to exploit power management features such as S3/S4. This was discussed in [1] and [2]. Changelog: - This version v5: Some redundant error messages were removed and the code was streamlined. v4: http://www.redhat.com/archives/libvir-list/2011-August/msg00316.html v3: http://www.redhat.com/archives/libvir-list/2011-August/msg00282.html v2: http://www.redhat.com/archives/libvir-list/2011-August/msg00238.html v1: http://thread.gmane.org/gmane.comp.emulators.libvirt/40886 References: -- [1] http://www.redhat.com/archives/libvir-list/2011-August/msg00248.html [2] http://www.redhat.com/archives/libvir-list/2011-August/msg00302.html Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com --- docs/formatcaps.html.in | 19 docs/schemas/capability.rng | 18 +++ include/libvirt/virterror.h |1 + libvirt.spec.in |2 ++ src/conf/capabilities.c | 27 +- src/conf/capabilities.h |4 +++ src/libvirt_private.syms |1 + src/qemu/qemu_capabilities.c |7 ++ src/util/util.c | 51 ++ src/util/util.h | 14 src/util/virterror.c |3 ++ 11 files changed, 141 insertions(+), 6 deletions(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index a4297ce..ce6f9a6 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -28,6 +28,10 @@ BIOS you will see/p lt;feature name='xtpr'/gt; ... lt;/cpugt; +lt;power_managementgt; + lt;S3/gt; + lt;S4/gt; +lt;power_management/gt; lt;/hostgt;/span lt;!-- xen-3.0-x86_64 --gt; @@ -61,11 +65,16 @@ BIOS you will see/p ... lt;/capabilitiesgt;/pre pThe first block (in red) indicates the host hardware capabilities, currently -it is limited to the CPU properties but other information may be available, -it shows the CPU architecture, topology, model name, and additional features -which are not included in the model but the CPU provides them. Features of the -chip are shown within the feature block (the block is similar to what you will -find in a Xen fully virtualized domain description)./p +it is limited to the CPU properties and the power management features of +the host platform, but other information may be available, it shows the CPU architecture, +topology, model name, and additional features which are not included in the model but the +CPU provides them. Features of the chip are shown within the feature block (the block is +similar to what you will find in a Xen fully virtualized domain description). Further, +the power management features supported by the host are shown, such as Suspend-to-RAM (S3) +and Suspend-to-Disk (S4). In case the query for power management features succeeded but the +host does not support any such feature, then an empty lt;power_management/gt; +tag will be shown. Otherwise, if the query itself failed, no such tag will +be displayed (i.e., there will not be any power_management block or empty tag in the XML)./p pThe second block (in blue) indicates the paravirtualization support of the Xen support, you will see the os_type of xen to indicate a paravirtual kernel, then architecture information and potential features./p diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index
Re: [libvirt] nwfilter - limit VM traffic to specific mac address
On 11/09/2011 04:01 AM, Shahar Havivi wrote: On 08.11.11 16:34, Stefan Berger wrote: On 11/07/2011 04:25 AM, Shahar Havivi wrote: Hi, I want to limit VM traffic to a specific MAC address, ie VMs cannot traffic each other other then a specific gateway. I am using custom nwfilter name: isolatedprivatevlan-vdsm.xml located in /etc/libvirt/nwfilter/: filter name='isolatedprivatevlan-vdsm' chain='root' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='500' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Try this one -- it works in 'my' subnet: filter name='isolatedprivatevlan-vdsm' chain='ipv4' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='10' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Thanks, Now it is blocking the traffic but I can't get traffic to the gateway as well... That's odd. Can you ping the gateway from the VM? Is it typically ping-able? Are you sure you specified the correct MAC addresses -- check with 'arp -n' on a host in the same subnet and see what it shows for the gateway (ping it if you don't see an entry). Stefan VM1 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:53/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface VM2 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:52/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface in each VM (Fedora 15 LiveCD) I assign ip: # ifconfig eth0 10.35.1.240 netmask 255.255.254.0 # route add default gw 10.35.1.1 vm2: # ifconfig eth0 10.35.1.241 netmask 255.255.254.0 # route add default gw 10.35.1.1 but the filter is not working, I can ping the VMs from each other, Am I missing something? Try the above filter that puts the check into a different 'chain' into different order. I'll be introducing a 'mac' chain where this can then be put into rather than into the 'ipv4' chain. The challenging part about the filtering rules is their order and the XML can unfortunately not abstract this 'away'. Stefan Thanks, Shahar Havivi. -- 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 16/33] Remove ifaceUp, ifaceDown, ifaceCtrl ifaceIsUp APIs
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com The ifaceUp, ifaceDown, ifaceCtrl ifaceIsUp APIs can be replaced with calls to virNetDevSetOnline and virNetDevIsOnline * src/util/interface.c, src/util/interface.h: Delete ifaceUp, ifaceDown, ifaceCtrl ifaceIsUp * src/nwfilter/nwfilter_gentech_driver.c, src/util/macvtap.c: Update to use virNetDevSetOnline and virNetDevIsOnline --- src/libvirt_private.syms |3 - src/nwfilter/nwfilter_gentech_driver.c |4 +- src/util/interface.c | 150 src/util/interface.h | 12 --- src/util/macvtap.c | 18 ++-- 5 files changed, 10 insertions(+), 177 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] Implement the asynchronous suspend and RTC wakeup
Add the core functions that implement the functionality of the API. Suspend is done by using an asynchronous mechanism so that we can return the status to the caller successfully before the host gets suspended. This asynchronous operation is achieved by suspending the host in a separate thread of execution. To resume the host, an RTC alarm is set up (based on how long we want to suspend) before suspending the host. When this alarm fires, the host gets woken up. Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in |5 + src/libvirt_private.syms |7 + src/nodeinfo.c | 220 ++ src/nodeinfo.h | 14 +++ src/qemu/qemu_driver.c |5 + src/util/threads-pthread.c | 17 +++ src/util/threads.h |1 7 files changed, 268 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..25f1c9b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3357,6 +3357,11 @@ typedef struct _virTypedParameter virMemoryParameter; */ typedef virMemoryParameter *virMemoryParameterPtr; +typedef enum { +VIR_S3 = 1, /* Suspend-to-RAM */ +VIR_S4 = 2 /* Suspend-to-disk */ +} virSuspendState; + #ifdef __cplusplus } #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6a1562e..2fa84e0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -844,6 +844,13 @@ nodeGetCellsFreeMemory; nodeGetFreeMemory; nodeGetInfo; nodeGetMemoryStats; +virSuspendLock; +virSuspendUnlock; +virSuspendInit; +nodeSuspendForDuration; +setNodeWakeup; +nodeSuspend; +virSuspendCleanup; # nwfilter_conf.h diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 6448b79..3c67fe6 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -46,6 +46,9 @@ #include count-one-bits.h #include intprops.h #include virfile.h +#include command.h +#include threads.h +#include datatypes.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -65,6 +68,11 @@ # define LINUX_NB_MEMORY_STATS_ALL 4 # define LINUX_NB_MEMORY_STATS_CELL 2 +/* Bitmask to hold the Power Management features supported by the host, + * such as Suspend-to-RAM (S3), Suspend-to-Disk (S4) etc. + */ +static unsigned int hostPMFeatures; + /* NB, this is not static as we need to call it from the testsuite */ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virNodeInfoPtr nodeinfo, @@ -897,3 +905,215 @@ unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) return 0; } #endif + + +static int initialized; +virMutex virSuspendMutex; + +int virSuspendLock(void) +{ +return virMutexTryLock(virSuspendMutex); +} + +void virSuspendUnlock(void) +{ +virMutexUnlock(virSuspendMutex); +} + +/** + * virSuspendInit: + * + * Get the low power states supported by the host, such as Suspend-to-RAM (S3) + * or Suspend-to-Disk (S4), so that a request to suspend/hibernate the host + * can be handled appropriately based on this information. + * + * Returns 0 if successful, and -1 in case of error. + */ +int virSuspendInit(void) +{ + +if (virMutexInit(virSuspendMutex) 0) +return -1; + +/* Get the power management capabilities supported by the host. + * Ensure that this is done only once, by using the 'initialized' + * variable. + */ +if (virGetPMCapabilities(hostPMFeatures) 0) { +VIR_ERROR(_(Failed to get host power management features)); +return -1; +} + +return 0; +} + + +/** + * nodeSuspendForDuration: + * @conn: pointer to the hypervisor connection + * @state: the state to which the host must be suspended to - + * VIR_HOST_PM_S3 (Suspend-to-RAM) + * or VIR_HOST_PM_S4 (Suspend-to-disk) + * @duration: the time duration in seconds, for which the host + *must be suspended + * + * Suspend the node (host machine) for the given duration of time + * in the specified state (such as S3 or S4). Resume the node + * after the time duration is complete. + * + * An RTC alarm is set appropriately to wake up the node from + * its sleep state. Then the actual node suspend is carried out + * asynchronously in another thread, after a short time delay + * so as to give enough time for this function to return status + * to its caller through the connection. + * + * Returns 0 in case the node is going to be suspended after a short + * delay, -1 if suspending the node is not supported, or if a + * previous suspend operation is still in progress. + */ +int nodeSuspendForDuration(virConnectPtr conn ATTRIBUTE_UNUSED, + int state, + unsigned long long duration) +{ +static virThread thread; +char *cmdString; + +if (!initialized) { +if(virSuspendInit() 0) +return -1; +initialized = 1; +} + +/* + * Ensure that we are the only ones trying
[libvirt] [PATCH v3 2/2] Make the API public
Define the required interfaces to export the API. Signed-off-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in |4 src/driver.h |5 src/libvirt.c| 48 ++ src/libvirt_public.syms |1 + src/qemu/qemu_driver.c |1 + src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 12 ++- 7 files changed, 71 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 25f1c9b..809a1fd 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1055,6 +1055,10 @@ unsigned long long virNodeGetFreeMemory (virConnectPtr conn); int virNodeGetSecurityModel (virConnectPtr conn, virSecurityModelPtr secmodel); +int virNodeSuspendForDuration (virConnectPtr conn, + int state, + unsigned long long duration); + /* * Gather list of running domains */ diff --git a/src/driver.h b/src/driver.h index 4c14aaa..981bfae 100644 --- a/src/driver.h +++ b/src/driver.h @@ -740,6 +740,10 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +typedef int +(*virDrvNodeSuspendForDuration)(virConnectPtr conn, int state, +unsigned long long duration); + /** * _virDriver: @@ -899,6 +903,7 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; +virDrvNodeSuspendForDuration nodeSuspendForDuration; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index b0d1e01..fc4575a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6303,6 +6303,54 @@ error: } /** + * virNodeSuspendForDuration: + * @conn: pointer to the hypervisor connection + * @state: the state to which the host must be suspended to, + * such as: VIR_S3 (Suspend-to-RAM) + * VIR_S4 (Suspend-to-Disk) + * @duration: the time duration in seconds, for which the host + *has to be suspended + * + * Suspend the node (host machine) for the given duration of time + * in the specified state (such as S3 or S4). Resume the node + * after the time duration is complete. + * + * Returns 0 on success (i.e., the node will be suspended after a + * short delay), -1 on failure (the operation is not supported). + */ +int +virNodeSuspendForDuration(virConnectPtr conn, + int state, + unsigned long long duration) +{ + +VIR_DEBUG(conn=%p, state=%d, duration=%lld, conn, state, duration); + +virResetLastError(); + +if (!VIR_IS_CONNECT(conn)) { +virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} + +if (conn-driver-nodeSuspendForDuration) { +int ret; +ret = conn-driver-nodeSuspendForDuration(conn, state, duration); +if (ret 0) +goto error; +return ret; +} + +virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(conn); +return -1; +} + + +/** * virDomainGetSchedulerType: * @domain: pointer to domain object * @nparams: pointer to number of scheduler parameters, can be NULL diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bcefb10..fd44170 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -478,6 +478,7 @@ LIBVIRT_0.9.4 { virDomainGetBlockJobInfo; virDomainBlockJobSetSpeed; virDomainBlockPull; +virNodeSuspendForDuration; } LIBVIRT_0.9.3; LIBVIRT_0.9.5 { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b4dc582..f744539 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10911,6 +10911,7 @@ static virDriver qemuDriver = { .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */ +.nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.7 */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f3b8ad5..2f6b29a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4526,6 +4526,7 @@ static virDriver remote_driver = { .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */ +.nodeSuspendForDuration = remoteNodeSuspendForDuration, /* 0.9.7 */ }; static virNetworkDriver network_driver = {
Re: [libvirt] make rpm failing with git head on RHEL6
On 09/11/2011, at 6:22 PM, Wen Congyang wrote: snip To clarify, systemtap-sdt-devel is already installed. Doing a brand new autogen.sh, then compiling normally with: I find the following messages from autogen.sh's output: running CONFIG_SHELL=/bin/sh /bin/sh ./configure --with-dtrace=no --no-create --no-recursion checking for a BSD-compatible install... /usr/bin/install -c You have disabled dtrace, so make -j 3 does not fail. Please run ./configure again, and check whether dtrace is enabled. Thanks, well spotted! :) Looks like I needed to run make distclean first, to get rid of old preferences. Things worked fine after that (both normal make, and make rpm). On another note, Eric, do you reckon it's possible to add the XHTML1 DTDs to maintainer requirements? Without them, generation of the html docs fails at the validation stage. i.e. for make rpm: ... missing XHTML1 DTD missing XHTML1 DTD cp: cannot stat `./api.html': No such file or directory make[2]: *** [distdir] Error 1 rm news.html.tmp goals.html.tmp archnetwork.html.tmp formatsecret.html.tmp drvopenvz.html.tmp windows.html.tmp testtck.html.tmp drvlxc.html.tmp apps.html.tmp docs.html.tmp formatstorageencryption.html.tmp drvvbox.html.tmp formatdomain.html.tmp php.html.tmp drvhyperv.html.tmp errors.html.tmp hacking.html.tmp format.html.tmp hooks.html.tmp api.html.tmp storage.html.tmp formatcaps.html.tmp relatedlinks.html.tmp compiling.html.tmp index.html.tmp archdomain.html.tmp downloads.html.tmp auth.html.tmp archnode.html.tmp hvsupport.html.tmp bugs.html.tmp java.html.tmp locking.html.tmp logging.html.tmp testapi.html.tmp formatnetwork.html.tmp bindings.html.tmp uri.html.tmp drvtest.html.tmp sitemap.html.tmp drvqemu.html.tmp contact.html.tmp formatnwfilter.html.tmp intro.html.tmp drvxen.html.tmp drvesx.html.tmp drvvmware.html.tmp testsuites.html.tmp devguide.html.tmp firewall.html.tmp architecture.html.tmp migration.html.tmp todo.html.tmp formatsnapshot.html.tmp formatstorage.html.tmp in! ternals.html.tmp deployment.html.tmp api_extension.html.tmp drvuml.html.tmp virshcmdref.html.tmp csharp.html.tmp python.html.tmp drvremote.html.tmp archstorage.html.tmp remote.html.tmp drivers.html.tmp formatnode.html.tmp make[2]: Leaving directory `/home/jc/git_repos/libvirt/docs' make[1]: *** [distdir] Error 1 make[1]: Leaving directory `/home/jc/git_repos/libvirt' make: *** [rpm] Error 2 In Fedora/RHEL it's the xhtml1-dtds (noarch) package. Regards and best wishes, Justin Clift Thanks Wen Congyang -- Aeolus Community Manager http://www.aeolusproject.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/2] API to invoke S3/S4 on a host and also resume from within libvirt
(This patch is positioned to go in after the patch that exports the host power management capabilities as XML, posted in [4]) This patch adds a new API to put a host to a suspended state (either Suspend-to-RAM or Suspend-to-Disk) and setup a timed resume to get the host back online, from within libvirt. This uses the RTC wakeup mechanism to set up a timer alarm before suspending the host, so that in-band resume is facilitated by the firing of the RTC alarm, which wakes up the host. The decision to use the RTC Wakeup mechanism to resume the host from sleep was taken in [1]. An initial API was discussed in [2]. Some design ideas for the asynchronous mechanism implementation was discussed in [3]. v3: * Rebased to libvirt 0.9.7 * Added a check to see if alarmTime (suspend duration) is within an acceptable range. v2: * Added an init function which finds out if S3/S4 is supported by the host, upon the first request to suspend/hibernate. * Added synchronization/locking to ensure that only one suspend operation is active at a time. v1: http://www.redhat.com/archives/libvir-list/2011-September/msg00830.html v2: http://comments.gmane.org/gmane.comp.emulators.libvirt/46729 References: [1]. http://www.redhat.com/archives/libvir-list/2011-August/msg00327.html [2]. http://www.redhat.com/archives/libvir-list/2011-August/msg00248.html [3]. http://www.redhat.com/archives/libvir-list/2011-September/msg00438.html [4]. http://www.redhat.com/archives/libvir-list/2011-November/msg00378.html Srivatsa S. Bhat (2): Implement the asynchronous suspend and RTC wakeup Make the API public include/libvirt/libvirt.h.in |9 ++ src/driver.h |5 + src/libvirt.c| 48 + src/libvirt_private.syms |7 + src/libvirt_public.syms |1 src/nodeinfo.c | 220 ++ src/nodeinfo.h | 14 +++ src/qemu/qemu_driver.c |6 + src/remote/remote_driver.c |1 src/remote/remote_protocol.x | 12 ++ src/util/threads-pthread.c | 17 +++ src/util/threads.h |1 12 files changed, 339 insertions(+), 2 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] nwfilter - limit VM traffic to specific mac address
On 09.11.11 06:44, Stefan Berger wrote: On 11/09/2011 04:01 AM, Shahar Havivi wrote: On 08.11.11 16:34, Stefan Berger wrote: On 11/07/2011 04:25 AM, Shahar Havivi wrote: Hi, I want to limit VM traffic to a specific MAC address, ie VMs cannot traffic each other other then a specific gateway. I am using custom nwfilter name: isolatedprivatevlan-vdsm.xml located in /etc/libvirt/nwfilter/: filter name='isolatedprivatevlan-vdsm' chain='root' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='500' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Try this one -- it works in 'my' subnet: filter name='isolatedprivatevlan-vdsm' chain='ipv4' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='10' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Thanks, Now it is blocking the traffic but I can't get traffic to the gateway as well... That's odd. Can you ping the gateway from the VM? Is it typically ping-able? Are you sure you specified the correct MAC addresses -- check with 'arp -n' on a host in the same subnet and see what it shows for the gateway (ping it if you don't see an entry). Stefan It's working only when I remove the line filterref filter='clean-traffic'/ from the filter... VM1 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:53/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface VM2 domian xml portion: interface type=bridge mac address=00:1a:4a:16:01:52/ model type=virtio/ source bridge=red/ filterref filter=isolatedprivatevlan-vdsm parameter name=GATEWAY_MAC value=00:00:0c:07:ac:00/ /filterref /interface in each VM (Fedora 15 LiveCD) I assign ip: # ifconfig eth0 10.35.1.240 netmask 255.255.254.0 # route add default gw 10.35.1.1 vm2: # ifconfig eth0 10.35.1.241 netmask 255.255.254.0 # route add default gw 10.35.1.1 but the filter is not working, I can ping the VMs from each other, Am I missing something? Try the above filter that puts the check into a different 'chain' into different order. I'll be introducing a 'mac' chain where this can then be put into rather than into the 'ipv4' chain. The challenging part about the filtering rules is their order and the XML can unfortunately not abstract this 'away'. Stefan Thanks, Shahar Havivi. -- 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
[libvirt] ANNOUNCE: Release 0.9.7 of Perl Bindings Sys::Virt
I am pleased to announce an update of the Perl bindings Sys::Virt, version 0.9.7 is now available for download: http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.9.7.tar.gz New in 0.9.7: - Add all new APIs in libvirt 0.9.7 - Add binding for virDomainOpenConsole - Fix missing $flags in documentation - Added example integrating Sys::Virt::Domain-open_graphics GTK3 and GTK-VNC The permalink for Sys-Virt, where all previous releases can also be obtained, is: http://search.cpan.org/dist/Sys-Virt/ Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] build: allow for local gnulib diffs
On Tue, Nov 08, 2011 at 05:37:19PM -0700, Eric Blake wrote: Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. While it would be nice to pull in a newer version of .gnulib that fixes this, it is difficult to backport any .gnulib update to older releases. So, it makes sense to take advantage of gnulib-tool's ability to support local diffs, where we can apply specific diffs in our use of gnulib without waiting for upstream gnulib to pick up those changes, as well as avoiding a wholesale .gnulib update. The existence of local diffs will also make it easier to backport fixes against a tarball (as long as a tarball and libvirt.git share the same .gnulib commit, then the tarball can be patched by applying the same local diffs as a post-release libvirt.git commit, without having to rerun an entire gnulib-tool bootstrap). This patch introduces the framework for supporting local diffs, without actually introducing any. * bootstrap.conf (local_gl_dir): New variable. * autogen.sh (bootstrap_hash): Hash any local diffs, to force a re-bootstrap if just diffs change. * cfg.mk (_update_required): Likewise. --- autogen.sh |3 +++ bootstrap.conf |6 -- cfg.mk |1 + 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/autogen.sh b/autogen.sh index b64521e..f1591d8 100755 --- a/autogen.sh +++ b/autogen.sh @@ -41,10 +41,13 @@ fi # is required. The first is just the SHA1 that selects a gnulib snapshot. # The second ensures that whenever we change the set of gnulib modules used # by this package, we rerun bootstrap to pull in the matching set of files. +# The third ensures that whenever we change the set of local gnulib diffs, +# we rerun bootstrap to pull in those diffs. bootstrap_hash() { git submodule status | sed 's/^[ +-]//;s/ .*//' git hash-object bootstrap.conf +git ls-tree -d HEAD gnulib/local | awk '{print $3}' } # Ensure that whenever we pull in a gnulib update or otherwise change to a diff --git a/bootstrap.conf b/bootstrap.conf index 4557d2d..6498aba 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -160,9 +160,10 @@ fi # Tell gnulib to: # require LGPLv2+ +# apply any local diffs in gnulib/local/ dir # put *.m4 files in new gnulib/m4/ dir -# put *.[ch] files in new gnulib/lib/ dir. -# import gnulib tests in new gnulib/tests/ dir. +# put *.[ch] files in new gnulib/lib/ dir +# import gnulib tests in new gnulib/tests/ dir gnulib_name=libgnu m4_base=gnulib/m4 source_base=gnulib/lib @@ -172,6 +173,7 @@ gnulib_tool_option_extras=\ --with-tests\ --avoid=pt_chown\ +local_gl_dir=gnulib/local # Convince bootstrap to use multiple m4 directories. : ${ACLOCAL=aclocal} diff --git a/cfg.mk b/cfg.mk index 463ce0c..574c7a4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -627,6 +627,7 @@ ifeq (0,$(MAKELEVEL)) test -f po/Makevars || { echo 1; exit; }; \ actual=$$(git submodule status | $(_submodule_hash); \ git hash-object bootstrap.conf; \ + git ls-tree -d HEAD gnulib/local | awk '{print $$3}'; \ git diff .gnulib); \ stamp=$$($(_submodule_hash) $(_curr_status) 2/dev/null);\ test $$stamp = $$actual; echo $$?) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] build: fix mingw build of gnulib openpty
On Tue, Nov 08, 2011 at 05:37:20PM -0700, Eric Blake wrote: Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. Work around it while waiting for an upstream gnulib fix. * gnulib/local/lib/pty.in.h (openpty): Provide forward declarations of opaque structs not present on mingw. * gnulib/local/lib/openpty.c (openpty): Provide stub for mingw. --- gnulib/local/lib/openpty.c.diff | 26 ++ gnulib/local/lib/pty.in.h.diff | 13 + 2 files changed, 39 insertions(+), 0 deletions(-) create mode 100644 gnulib/local/lib/openpty.c.diff create mode 100644 gnulib/local/lib/pty.in.h.diff diff --git a/gnulib/local/lib/openpty.c.diff b/gnulib/local/lib/openpty.c.diff new file mode 100644 index 000..f17e566 --- /dev/null +++ b/gnulib/local/lib/openpty.c.diff @@ -0,0 +1,26 @@ +diff --git c/lib/openpty.c i/lib/openpty.c +index c398db5..d61d5ba 100644 +--- c/lib/openpty.c i/lib/openpty.c +@@ -32,6 +32,21 @@ rpl_openpty (int *amaster, int *aslave, char *name, + (struct winsize *) winp); + } + ++#elif (defined _WIN32 || defined __WIN32__) !defined __CYGWIN__ /* mingw */ ++ ++# include errno.h ++ ++int ++openpty (int *amaster _GL_UNUSED, int *aslave _GL_UNUSED, ++ char *name _GL_UNUSED, ++ struct termios const *termp _GL_UNUSED, ++ struct winsize const *winp _GL_UNUSED) ++{ ++ /* Mingw lacks pseudo-terminals altogether. */ ++ errno = ENOSYS; ++ return -1; ++} ++ + #else /* AIX 5.1, HP-UX 11, IRIX 6.5, Solaris 10, mingw */ + + # include fcntl.h diff --git a/gnulib/local/lib/pty.in.h.diff b/gnulib/local/lib/pty.in.h.diff new file mode 100644 index 000..9470700 --- /dev/null +++ b/gnulib/local/lib/pty.in.h.diff @@ -0,0 +1,13 @@ +diff --git c/lib/pty.in.h i/lib/pty.in.h +index aff989c..00eecc6 100644 +--- c/lib/pty.in.h i/lib/pty.in.h +@@ -92,6 +92,8 @@ _GL_WARN_ON_USE (forkpty, forkpty is not declared consistently - + /* Create pseudo tty master slave pair and set terminal attributes +according to TERMP and WINP. Return handles for both ends in +*AMASTER and *ASLAVE, and return the name of the slave end in NAME. */ ++struct termios; ++struct winsize; + # if @REPLACE_OPENPTY@ + # if !(defined __cplusplus defined GNULIB_NAMESPACE) + # undef openpty -- 1.7.4.4 ACK, these fix the mingw32 build for me Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix 0.9.7 for mingw
On Tue, Nov 08, 2011 at 05:37:18PM -0700, Eric Blake wrote: I'm proposing two options to fix the build failure mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg00309.html without having to cut a release of 0.9.8. BTW, slightly related, so see a non-fatal compile warning from gnulib on mingw32 too which i presume wants fixing upstream: CC sigaction.lo CC sigprocmask.lo sigprocmask.c: In function '_gl_raise_SIGPIPE': sigprocmask.c:349:1: warning: control reaches end of non-void function CC sleep.lo CC snprintf.lo Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Don't return a fatal error if receiving unexpected stream data
From: Daniel P. Berrange berra...@redhat.com Due to the asynchronous nature of streams, we might continue to receive some stream packets from the server even after we have shutdown the stream on the client side. These should be discarded silently, rather than raising an error in the RPC layer. * src/rpc/virnetclient.c: Discard stream data silently --- src/rpc/virnetclient.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 4b7d4a9..4cbdf55 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -621,7 +621,9 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) VIR_DEBUG(No stream found for packet with prog=%d vers=%d serial=%u proc=%u, client-msg.header.prog, client-msg.header.vers, client-msg.header.serial, client-msg.header.proc); -return -1; +/* Don't return -1, because we expect to see further stream packets + * after we've shut it down sometimes */ +return 0; } /* Finish/Abort are synchronous, so also see if there's an -- 1.7.6.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] nwfilter - limit VM traffic to specific mac address
On 09.11.11 09:20, Stefan Berger wrote: On 11/09/2011 07:44 AM, Shahar Havivi wrote: On 09.11.11 06:44, Stefan Berger wrote: On 11/09/2011 04:01 AM, Shahar Havivi wrote: On 08.11.11 16:34, Stefan Berger wrote: On 11/07/2011 04:25 AM, Shahar Havivi wrote: Hi, I want to limit VM traffic to a specific MAC address, ie VMs cannot traffic each other other then a specific gateway. I am using custom nwfilter name: isolatedprivatevlan-vdsm.xml located in /etc/libvirt/nwfilter/: filter name='isolatedprivatevlan-vdsm' chain='root' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='500' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Try this one -- it works in 'my' subnet: filter name='isolatedprivatevlan-vdsm' chain='ipv4' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='10' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Thanks, Now it is blocking the traffic but I can't get traffic to the gateway as well... That's odd. Can you ping the gateway from the VM? Is it typically ping-able? Are you sure you specified the correct MAC addresses -- check with 'arp -n' on a host in the same subnet and see what it shows for the gateway (ping it if you don't see an entry). Stefan It's working only when I remove the line filterref filter='clean-traffic'/ from the filter... While you ping the gateway, can you re-add the above line to the filter? Stefan its working, even when stopping the ping and re-pinging the gateway, but it stop working after I stop and started the VM. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Virtual serial logging server?
Hi, 2011/11/6 Reeted ree...@shiftmail.org: Dear all, please excuse the almost-OT question, I see various possibilities in quemu-kvm and libvirt for sending virtual serial port data to files, sockets, pipes, etc on the host. In particular, the TCP socket seems interesting. Can you suggest a server application to receive all such TCP connections and log serial data for many virtual machines at once? In particular I would be interested in something with quotas, i.e. something that deletes old lines from the logs of a certain VM when the filesystem space occupied by the serial logs of such VM gets over a certain amount of space. So that the log space for other VMs is not starved in case one of them loops. May be you could build something around conserver: http://www.conserver.com Bests -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't return a fatal error if receiving unexpected stream data
On 11/09/2011 06:55 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Due to the asynchronous nature of streams, we might continue to receive some stream packets from the server even after we have shutdown the stream on the client side. These should be discarded silently, rather than raising an error in the RPC layer. * src/rpc/virnetclient.c: Discard stream data silently --- src/rpc/virnetclient.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 4b7d4a9..4cbdf55 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -621,7 +621,9 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) VIR_DEBUG(No stream found for packet with prog=%d vers=%d serial=%u proc=%u, client-msg.header.prog, client-msg.header.vers, client-msg.header.serial, client-msg.header.proc); -return -1; +/* Don't return -1, because we expect to see further stream packets + * after we've shut it down sometimes */ +return 0; ACK. -- 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] nwfilter - limit VM traffic to specific mac address
On 11/09/2011 09:38 AM, Shahar Havivi wrote: On 09.11.11 09:20, Stefan Berger wrote: On 11/09/2011 07:44 AM, Shahar Havivi wrote: On 09.11.11 06:44, Stefan Berger wrote: On 11/09/2011 04:01 AM, Shahar Havivi wrote: On 08.11.11 16:34, Stefan Berger wrote: On 11/07/2011 04:25 AM, Shahar Havivi wrote: Hi, I want to limit VM traffic to a specific MAC address, ie VMs cannot traffic each other other then a specific gateway. I am using custom nwfilter name: isolatedprivatevlan-vdsm.xml located in /etc/libvirt/nwfilter/: filter name='isolatedprivatevlan-vdsm' chain='root' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='500' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Try this one -- it works in 'my' subnet: filter name='isolatedprivatevlan-vdsm' chain='ipv4' filterref filter='clean-traffic'/ rule action='drop' direction='out' priority='10' mac match='no' dstmacaddr='$GATEWAY_MAC'/ /rule /filter Thanks, Now it is blocking the traffic but I can't get traffic to the gateway as well... That's odd. Can you ping the gateway from the VM? Is it typically ping-able? Are you sure you specified the correct MAC addresses -- check with 'arp -n' on a host in the same subnet and see what it shows for the gateway (ping it if you don't see an entry). Stefan It's working only when I remove the line filterref filter='clean-traffic'/ from the filter... While you ping the gateway, can you re-add the above line to the filter? Stefan its working, even when stopping the ping and re-pinging the gateway, but it stop working after I stop and started the VM. How does the VM get its IP address, static or DHCP ? If DHCP, could you try a static IP address? In case it doesn't work, what does 'ebtables -t nat -L' show and which IP address is assigned to the VM's interface? Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow non-blocking message sending on virNetClient
On Tue, Nov 08, 2011 at 18:20:02 +, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Split the existing virNetClientSend into two parts virNetClientSend and virNetClientSendNoReply, instead of having a 'bool expectReply' parameter. Add a new virNetClientSendNonBlock which returns 2 on full send, 1 on partial send, 0 on no send, -1 on error If a partial send occurs, then a subsequent call to any of the virNetClientSend* APIs will finish any outstanding I/O. Do I understand it correctly that the main difference between this patch and my patch is that my patch stores the unfinished call separated from the other call while you store it at the beginning of the waitDispatch list? Your solution looks cleaner but I suspect the other minor differences from my patch are actually bugs (more on them later in this email) :-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 4b7d4a9..b0ed507 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c ... @@ -924,6 +954,12 @@ static int virNetClientIOEventLoop(virNetClientPtr client, if (virNetSocketHasCachedData(client-sock)) timeout = 0; +/* If we're a non-blocking call, then we don't + * want to wait for I/O readyness + */ +if (thiscall-nonBlock) +timeout = 0; + fds[0].events = fds[0].revents = 0; fds[1].events = fds[1].revents = 0; @@ -975,8 +1011,34 @@ static int virNetClientIOEventLoop(virNetClientPtr client, /* If we have existing SASL decoded data, pretend * the socket became readable so we consume it */ -if (virNetSocketHasCachedData(client-sock)) +if (virNetSocketHasCachedData(client-sock)) { fds[0].revents |= POLLIN; +} else if (ret == 0 thiscall-nonBlock) { +if (thiscall-msg-bufferOffset == 0) { Unfortunately, this condition won't work with SASL since it encodes some data first time we call virNetClientIOWriteMessage but returns 0 and requires us to call it repeatedly until it returns bufferLength, which means all data were sent. Thus, thiscall-msg-bufferOffset == 0 doesn't mean we did not send any data. +/* No data sent at all, remove ourselves from the list */ +tmp = client-waitDispatch; +prev = NULL; +while (tmp) { +if (tmp == thiscall) { +if (prev) { +prev-next = thiscall-next; +} else { +client-waitDispatch = thiscall-next; +} +break; +} +prev = tmp; +tmp = tmp-next; +} +VIR_DEBUG(Giving up the buck %p %p, thiscall, client-waitDispatch); +virNetClientPassTheBuck(client); +return 0; +} else { +VIR_DEBUG(Giving up the buck %p %p, thiscall, client-waitDispatch); +virNetClientPassTheBuck(client); +return 1; /* partial send */ +} +} if (fds[1].revents) { VIR_DEBUG(Woken up from poll by other thread); ... @@ -1117,20 +1196,27 @@ static int virNetClientIO(virNetClientPtr client, thiscall-msg-bufferLength, client-waitDispatch); +/* Trivially detect blocking if someone else has the buck already */ +if (client-haveTheBuck +thiscall-nonBlock) +return 0; Hmm, I don't think this is correct. We actually want nonBlock call to be send while another thread has the buck and is waiting for its call to finish. The call may take a long time to complete and we need to send keepalive requests/responses during that time. + +/* Stick ourselves on the end of the wait queue */ +tmp = client-waitDispatch; +while (tmp tmp-next) +tmp = tmp-next; +if (tmp) +tmp-next = thiscall; +else +client-waitDispatch = thiscall; + /* Check to see if another thread is dispatching */ -if (client-waitDispatch) { -/* Stick ourselves on the end of the wait queue */ -virNetClientCallPtr tmp = client-waitDispatch; +if (client-haveTheBuck) { char ignore = 1; -while (tmp tmp-next) -tmp = tmp-next; -if (tmp) -tmp-next = thiscall; -else -client-waitDispatch = thiscall; /* Force other thread to wakeup from poll */ if (safewrite(client-wakeupSendFD, ignore, sizeof(ignore)) != sizeof(ignore)) { +/* Something went wrong, so we need to remove that call we just added */ if (tmp) tmp-next = NULL; else ... Jirka -- libvir-list mailing list
Re: [libvirt] [PATCH 0/2] Fix 0.9.7 for mingw
On 11/09/2011 06:48 AM, Daniel P. Berrange wrote: On Tue, Nov 08, 2011 at 05:37:18PM -0700, Eric Blake wrote: I'm proposing two options to fix the build failure mentioned here: https://www.redhat.com/archives/libvir-list/2011-November/msg00309.html without having to cut a release of 0.9.8. BTW, slightly related, so see a non-fatal compile warning from gnulib on mingw32 too which i presume wants fixing upstream: CC sigaction.lo CC sigprocmask.lo sigprocmask.c: In function '_gl_raise_SIGPIPE': sigprocmask.c:349:1: warning: control reaches end of non-void function Thanks for pointing that out, and yes I'll fix it upstream in gnulib. -- 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/2] build: fix mingw build of gnulib openpty
On 11/09/2011 06:46 AM, Daniel P. Berrange wrote: On Tue, Nov 08, 2011 at 05:37:20PM -0700, Eric Blake wrote: Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. Work around it while waiting for an upstream gnulib fix. * gnulib/local/lib/pty.in.h (openpty): Provide forward declarations of opaque structs not present on mingw. * gnulib/local/lib/openpty.c (openpty): Provide stub for mingw. ACK, these fix the mingw32 build for me Pushed. -- 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 1/2] build: allow for local gnulib diffs
On 11/09/2011 06:47 AM, Daniel P. Berrange wrote: On Tue, Nov 08, 2011 at 05:37:19PM -0700, Eric Blake wrote: Commit f7bd00c12 pulled in a gnulib module that fails to compile on mingw. While it would be nice to pull in a newer version of .gnulib that fixes this, it is difficult to backport any .gnulib update to older releases. So, it makes sense to take advantage of gnulib-tool's ability to support local diffs, where we can apply specific diffs in our use of gnulib without waiting for upstream gnulib to pick up those changes, as well as avoiding a wholesale .gnulib update. The existence of local diffs will also make it easier to backport fixes against a tarball (as long as a tarball and libvirt.git share the same .gnulib commit, then the tarball can be patched by applying the same local diffs as a post-release libvirt.git commit, without having to rerun an entire gnulib-tool bootstrap). This patch introduces the framework for supporting local diffs, without actually introducing any. ACK Now pushed. -- 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 17/33] Remove ifaceSetMac and ifaceGetMac APIs
On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: The ifaceSetMac and ifaceGetMac APIs duplicate the functionality of the virNetDevSetMAC and virNetDevGetMAC APIs, but returning errno's instead of raising errors. * src/util/interface.c, src/util/interface.h: Remove ifaceSetMac and ifaceGetMac APIs, adjusting callers for new error behaviour --- src/libvirt_private.syms |2 - src/util/interface.c | 190 +++--- src/util/interface.h |4 - 3 files changed, 27 insertions(+), 169 deletions(-) Everything seems in order, and make make check make syntax-check passes. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/33] Make all brXXX APIs raise errors, instead of returning errnos
On 11/03/2011 01:29 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Currently every caller of the brXXX APIs has to store the returned errno value and then raise an error message. This results in inconsistent error messages across drivers, additional burden on the callers and makes the error reporting inaccurate since it is hard to distinguish different scenarios from 1 errno value. While making the message more exact, it could also make it more difficult to determine the context of the error. Sometimes it's useful to have both (a drop-down list with a stack trace embedded into each error message would be so nice to have sometimes :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix domjobabort regression
This reverts commit ef1065cf5ac; see also this bug report: https://bugzilla.redhat.com/show_bug.cgi?id=751900 In qemu 0.15.1 and earlier, during migration to file, the qemu_savevm_state_begin and qemu_savevm_state_iterate methods will both process as much migration data as possible until either 1. The file descriptor returns EAGAIN 2. The bandwidth rate limit is reached If we set the rate limit to ULONG_MAX, test 2 never becomes true. We're passing a plain file descriptor to QEMU and POSIX does not support EAGAIN on regular files / block devices, so test 1 never becomes true either. In the 'virsh save --bypass-cache' case, we pass a pipe instead of a regular fd, but using a pipe adds I/O overhead, so always passing a pipe just so qemu can see EAGAIN doesn't seem nice. The ultimate fix needs to come from qemu - background migration must respect asynchronous abort requests, or else periodically return control to the main handling loop without an EAGAIN and without waiting to hit an insanely large amount of data. But until a version of qemu is fixed to support unlimited data rates while still allowing cancellation, the best we can do is avoid the automatic use of unlimited rates from within libvirt (users can still explicitly change the migration rates, if they are aware that they are giving up the ability to cancel a job). Reverting the lone use of QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX is the simplest patch; this slows migration back down to a default 32M/sec cap, but also ensures that the main qemu processing loop will still be responsive to cancellation requests. Hopefully upstream qemu will provide us a means of safely using unlimited speed, including a runtime probe of that capability. * src/qemu/qemu_migration.c (qemuMigrationToFile): Revert attempt to use unlimited migration bandwidth when migrating to file. Signed-off-by: Daniel Veillard veill...@redhat.com Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_migration.c | 17 - 1 files changed, 0 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 838a31f..30f805d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2825,16 +2825,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, bool restoreLabel = false; virCommandPtr cmd = NULL; int pipeFD[2] = { -1, -1 }; -unsigned long saveMigBandwidth = priv-migMaxBandwidth; - -/* Increase migration bandwidth to unlimited since target is a file. - * Failure to change migration speed is not fatal. */ -if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { -qemuMonitorSetMigrationSpeed(priv-mon, - QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX); -priv-migMaxBandwidth = QEMU_DOMAIN_FILE_MIG_BANDWIDTH_MAX; -qemuDomainObjExitMonitorWithDriver(driver, vm); -} if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD) (!compressor || pipe(pipeFD) == 0)) { @@ -2946,13 +2936,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, ret = 0; cleanup: -/* Restore max migration bandwidth */ -if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { -qemuMonitorSetMigrationSpeed(priv-mon, saveMigBandwidth); -priv-migMaxBandwidth = saveMigBandwidth; -qemuDomainObjExitMonitorWithDriver(driver, vm); -} - VIR_FORCE_CLOSE(pipeFD[0]); VIR_FORCE_CLOSE(pipeFD[1]); virCommandFree(cmd); -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Virtual serial logging server?
On Wed, Nov 09, 2011 at 04:15:57PM +0100, Jérémie Tarot wrote: Hi, 2011/11/6 Reeted ree...@shiftmail.org: Dear all, please excuse the almost-OT question, I see various possibilities in quemu-kvm and libvirt for sending virtual serial port data to files, sockets, pipes, etc on the host. In particular, the TCP socket seems interesting. Can you suggest a server application to receive all such TCP connections and log serial data for many virtual machines at once? In particular I would be interested in something with quotas, i.e. something that deletes old lines from the logs of a certain VM when the filesystem space occupied by the serial logs of such VM gets over a certain amount of space. So that the log space for other VMs is not starved in case one of them loops. May be you could build something around conserver: http://www.conserver.com I've done a PoC implementation of conserver, and it works nicely. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/33] Make all brXXX APIs raise errors, instead of returning errnos
On Tue, Nov 08, 2011 at 09:09:31AM -0500, Stefan Berger wrote: On 11/03/2011 01:29 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Currently every caller of the brXXX APIs has to store the returned errno value and then raise an error message. This results in inconsistent error messages across drivers, additional burden on the callers and makes the error reporting inaccurate since it is hard to distinguish different scenarios from 1 errno value. * src/util/bridge.c: Raise errors instead of returning errnos * src/lxc/lxc_driver.c, src/network/bridge_driver.c, src/qemu/qemu_command.c, src/uml/uml_conf.c, src/uml/uml_driver.c: Remove error reporting code --- po/POTFILES.in |1 + src/lxc/lxc_driver.c|7 +- src/network/bridge_driver.c | 78 +++--- src/qemu/qemu_command.c | 23 + src/uml/uml_conf.c | 28 +- src/uml/uml_driver.c| 14 +-- src/util/bridge.c | 262 +-- 7 files changed, 196 insertions(+), 217 deletions(-) @@ -188,6 +217,8 @@ cleanup: int brDeleteBridge(const char *brname ATTRIBUTE_UNUSED) { +virReportSystemError(errno, + _(Unable to delete bridge %s), brname); errno - ENOSYS Fixed this and a few others, and the places where they reoccur in later patches. @@ -211,15 +242,17 @@ brAddInterface(const char *brname, struct ifreq ifr; if ((fd = brSetupControl(brname,ifr)) 0) -return errno; +return -1; if (!(ifr.ifr_ifindex = if_nametoindex(ifname))) { -ret = errno; +virReportSystemError(errno, + _(Unable to get interface index for %s), ifname); I don't see an errno being defined for if_nametoindex in case of failure. Use ENODEV ? Yeah I see it just returns 0, so I used ENODEV Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/33] Turn two int parameters into bools in bridge APIs
On Tue, Nov 08, 2011 at 09:21:19AM -0500, Stefan Berger wrote: On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com * src/util/bridge.c, src/util/bridge.h: s/int/bool/ in virNetDevSetOnline and virNetDevBridgeSetSTP --- src/util/bridge.c | 18 +- src/util/bridge.h |8 2 files changed, 13 insertions(+), 13 deletions(-) @@ -98,10 +98,10 @@ int virNetDevBridgeGetSTPDelay(const char *brname, int *delay) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevBridgeSetSTP(const char *brname, - int enable) + bool enable) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevBridgeGetSTP(const char *brname, - int *enable) + bool *enable) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevTapCreate(char **ifname, Hm, would have thought to see the callers being changed as well... Yes, there was one caller which needed to change from 0/1 to false/true Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/33] Remove usage of brctl command line tool
On Tue, Nov 08, 2011 at 09:48:34AM -0500, Stefan Berger wrote: On 11/03/2011 01:30 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Convert the virNetDevBridgeSetSTP and virNetDevBridgeSetSTPDelay to use ioctls instead of spawning brctl. Implement the virNetDevBridgeGetSTP and virNetDevBridgeGetSTPDelay methods which were declared in the header but never existed * src/util/bridge.c: Convert to use bridge ioctls instead of brctl --- configure.ac |2 - libvirt.spec.in |2 - src/util/bridge.c | 199 + 3 files changed, 184 insertions(+), 19 deletions(-) @@ -817,22 +924,54 @@ cleanup: int virNetDevBridgeSetSTPDelay(const char *brname, int delay) { -virCommandPtr cmd; +int fd = -1; Also applicable for the other patches, the initialization to -1 doesn't seem necessary. I prefer to be overly cautious when declaring file descriptor variables. Even though the current code will always initialize FD, someone might come along later and refactor the method, resulting in code which needs the -1 initialization at declaration. GCC is not entirely reliable at detecting use of uninitialized variables. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/33] Santize naming of socket address APIs
On Tue, Nov 08, 2011 at 08:50:41PM -0500, Laine Stump wrote: make check fails to build after applying this patch: CC sockettest.o sockettest.c:28:27: fatal error: virsocketaddr.h: No such file or directory compilation terminated. diff --git a/tests/sockettest.c b/tests/sockettest.c index b9e37ab..bcc2800 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -25,7 +25,7 @@ #includestdlib.h #includestring.h Here's the problem. util/network.[ch] split into multiple files in patch 13/33, but you've accidentally changed the name of the #include here in 09/33: -#include network.h +#include virsocketaddr.h #include testutils.h #include logging.h #include memory.h diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index ece611a..51f75b4 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -33,7 +33,7 @@ #include logging.h #include virfile.h #include command.h Same problem here: -#include network.h +#include virsocketaddr.h #include gnutls_1_0_compat.h ACK with those two #include lines changed back to network.h Yes, I squashed a later patch into this one and squashed too much ! Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/33] Rename all brXXXX APIs to follow new convention
On Tue, Nov 08, 2011 at 09:17:21AM -0500, Stefan Berger wrote: On 11/03/2011 01:29 PM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com The existing brXXX APIs in src/util/bridge.h are renamed to follow one of three different conventions - virNetDevXXX - operations for any type of interface - virNetDevBridgeXXX - operations for bridge interfaces - virNetDevTapXXX- operations for tap interfaces * src/util/bridge.h, src/util/bridge.c: Rename all APIs * src/lxc/lxc_driver.c, src/network/bridge_driver.c, src/qemu/qemu_command.c, src/uml/uml_conf.c, src/uml/uml_driver.c: Update for API renaming --- src/lxc/lxc_driver.c|2 +- src/network/bridge_driver.c | 32 src/qemu/qemu_command.c |4 +- src/uml/uml_conf.c |4 +- src/uml/uml_driver.c|2 +- src/util/bridge.c | 196 +++--- src/util/bridge.h | 74 7 files changed, 148 insertions(+), 166 deletions(-) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int brAddInetAddress(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevSetIPv4Addres(const char *ifname, Addres-Address Yes, and i've fixed it in later patches in the series where we move the functions to new files too. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/33] Refactor all network device handling code
On Thu, Nov 03, 2011 at 05:29:56PM +, Daniel P. Berrange wrote: This series is a major re-arrangement and de-duplication of internal code for dealing with physical network interfaces. Thanks for the reviews so far. I have pushed patches 01-10, with the relevant review fixes applied. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] nwfilter: avoid failure with noexec /tmp
If /tmp is mounted with the noexec flag (common on security-conscious systems), then nwfilter will fail to initialize, because we cannot run any temporary script via virRun(/tmp/script); but we _can_ use /bin/sh /tmp/script. For that matter, using /tmp risks collisions with other unrelated programs; we already have /var/run/libvirt as a dedicated temporary directory for use by libvirt. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Use internal directory, not /tmp; drop attempts to make script executable; and detect close error. (ebiptablesExecCLI): Switch to virCommand, and invoke the shell to read the script, rather than requiring an executable script. --- src/nwfilter/nwfilter_ebiptables_driver.c | 76 - 1 files changed, 21 insertions(+), 55 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index f87cfa1..c9c194c 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -40,6 +40,7 @@ #include nwfilter_ebiptables_driver.h #include virfile.h #include command.h +#include configmake.h #define VIR_FROM_THIS VIR_FROM_NWFILTER @@ -2482,29 +2483,17 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, * NULL in case of error with the error reported. * * Write the string into a temporary file and return the name of - * the temporary file. The string is assumed to contain executable - * commands. A line '#!/bin/sh' will automatically be written - * as the first line in the file. The permissions of the file are - * set so that the file can be run as an executable script. + * the temporary file. The file can then be read as a /bin/sh script. + * No '#!/bin/sh' header is needed, since the file will be read and not + * directly executed. */ static char * ebiptablesWriteToTempFile(const char *string) { -char filename[] = /tmp/virtdXX; -int len; +char filename[] = LOCALSTATEDIR /run/libvirt/nwfilt-XX; +size_t len; char *filnam; -virBuffer buf = VIR_BUFFER_INITIALIZER; -char *header; size_t written; -virBufferAddLit(buf, #!/bin/sh\n); - -if (virBufferError(buf)) { -virBufferFreeAndReset(buf); -virReportOOMError(); -return NULL; -} -header = virBufferContentAndReset(buf); - int fd = mkstemp(filename); if (fd 0) { @@ -2514,15 +2503,8 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -if (fchmod(fd, S_IXUSR| S_IRUSR | S_IWUSR) 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot change permissions on temp. file)); -goto err_exit; -} - -len = strlen(header); -written = safewrite(fd, header, len); +len = strlen(string); +written = safewrite(fd, string, len); if (written != len) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -2530,9 +2512,7 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -len = strlen(string); -written = safewrite(fd, string, len); -if (written != len) { +if (VIR_CLOSE(fd) 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot write string to file)); @@ -2545,12 +2525,9 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -VIR_FREE(header); -VIR_FORCE_CLOSE(fd); return filnam; err_exit: -VIR_FREE(header); VIR_FORCE_CLOSE(fd); unlink(filename); return NULL; @@ -2564,7 +2541,7 @@ err_exit: * @status: Pointer to an integer for returning the WEXITSTATUS of the *commands executed via the script the was run. * - * Returns 0 in case of success, != 0 in case of an error. The returned + * Returns 0 in case of success, 0 in case of an error. The returned * value is NOT the result of running the commands inside the shell * script. * @@ -2577,53 +2554,42 @@ ebiptablesExecCLI(virBufferPtr buf, { char *cmds; char *filename; -int rc; -const char *argv[] = {NULL, NULL}; +int rc = -1; +virCommandPtr cmd; if (virBufferError(buf)) { virReportOOMError(); virBufferFreeAndReset(buf); -return 1; +return -1; } *status = 0; cmds = virBufferContentAndReset(buf); - VIR_DEBUG(%s, NULLSTR(cmds)); - if (!cmds) return 0; filename = ebiptablesWriteToTempFile(cmds); -VIR_FREE(cmds); - if (!filename) -return 1; +goto cleanup; -argv[0] = filename; +cmd = virCommandNew(/bin/sh); +virCommandAddArg(cmd, filename); virMutexLock(execCLIMutex); -rc = virRun(argv, status); +rc = virCommandRun(cmd, status); virMutexUnlock(execCLIMutex); -if (rc == 0) { -if
[libvirt] [PATCH 0/2] fix nwfilter when /tmp is mounted noexec
https://bugzilla.redhat.com/show_bug.cgi?id=752254 points out that libvirt cannot support nwfilter on a system with /tmp mounted noexec (which is a very common setup in security-conscious setups), all because we were trying to directly invoke a temporary script instead of invoking a shell to read the script. I've split this patch into 2 parts, on the off-chance that patch 2 would run afoul of command line length limits (if the total size of the generated nwfilter commands could possibly cause E2BIG, then we have to go through a temporary file). But my recollection is that modern Linux kernels support unlimited command-line length (that is, ARG_MAX is not a concern on Linux), and that nwfilter_ebiptables_driver only compiles on Linux, so my preference would be to squash these into a single commit, if others agree that we don't have to worry about length limits. At any rate, I'm quite impressed at the number of lines of code I was able to remove in order to fix a bug! Eric Blake (2): nwfilter: avoid failure with noexec /tmp nwfilter: simplify execution of ebiptables scripts src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++-- 1 files changed, 10 insertions(+), 124 deletions(-) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] nwfilter: simplify execution of ebiptables scripts
It's not worth even worrying about a temporary file, unless we ever expect the script to exceed maximum command-line argument length limits. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI): Run the commands as an argument to /bin/sh, rather than worrying about a temporary file. (ebiptablesWriteToTempFile): Delete unused function. --- src/nwfilter/nwfilter_ebiptables_driver.c | 88 +--- 1 files changed, 4 insertions(+), 84 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index c9c194c..aacbd02 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2476,65 +2476,6 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, /** - * ebiptablesWriteToTempFile: - * @string : the string to write into the file - * - * Returns the tempory filename where the string was written into, - * NULL in case of error with the error reported. - * - * Write the string into a temporary file and return the name of - * the temporary file. The file can then be read as a /bin/sh script. - * No '#!/bin/sh' header is needed, since the file will be read and not - * directly executed. - */ -static char * -ebiptablesWriteToTempFile(const char *string) { -char filename[] = LOCALSTATEDIR /run/libvirt/nwfilt-XX; -size_t len; -char *filnam; -size_t written; - -int fd = mkstemp(filename); - -if (fd 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot create temporary file)); -goto err_exit; -} - -len = strlen(string); -written = safewrite(fd, string, len); -if (written != len) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot write string to file)); -goto err_exit; -} - -if (VIR_CLOSE(fd) 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot write string to file)); -goto err_exit; -} - -filnam = strdup(filename); -if (!filnam) { -virReportOOMError(); -goto err_exit; -} - -return filnam; - -err_exit: -VIR_FORCE_CLOSE(fd); -unlink(filename); -return NULL; -} - - -/** * ebiptablesExecCLI: * @buf : pointer to virBuffer containing the string with the commands to *execute. @@ -2546,36 +2487,20 @@ err_exit: * script. * * Execute a sequence of commands (held in the given buffer) as a /bin/sh - * script and return the status of the execution. + * script and return the status of the execution in *status (if status is + * NULL, then the script must exit with status 0). */ static int ebiptablesExecCLI(virBufferPtr buf, int *status) { -char *cmds; -char *filename; int rc = -1; virCommandPtr cmd; -if (virBufferError(buf)) { -virReportOOMError(); -virBufferFreeAndReset(buf); -return -1; -} - *status = 0; -cmds = virBufferContentAndReset(buf); -VIR_DEBUG(%s, NULLSTR(cmds)); -if (!cmds) -return 0; - -filename = ebiptablesWriteToTempFile(cmds); -if (!filename) -goto cleanup; - -cmd = virCommandNew(/bin/sh); -virCommandAddArg(cmd, filename); +cmd = virCommandNewArgList(/bin/sh, -c, NULL); +virCommandAddArgBuffer(cmd, buf); virMutexLock(execCLIMutex); @@ -2583,11 +2508,6 @@ ebiptablesExecCLI(virBufferPtr buf, virMutexUnlock(execCLIMutex); -unlink(filename); -VIR_FREE(filename); - -cleanup: -VIR_FREE(cmds); virCommandFree(cmd); return rc; -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] fix nwfilter when /tmp is mounted noexec
On 11/09/2011 10:46 AM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=752254 points out that libvirt cannot support nwfilter on a system with /tmp mounted noexec (which is a very common setup in security-conscious setups), all because we were trying to directly invoke a temporary script instead of invoking a shell to read the script. I've split this patch into 2 parts, on the off-chance that patch 2 would run afoul of command line length limits (if the total size of the generated nwfilter commands could possibly cause E2BIG, then we have to go through a temporary file). But my recollection is that modern Linux kernels support unlimited command-line length (that is, ARG_MAX is not a concern on Linux), and that nwfilter_ebiptables_driver only compiles on Linux, so my preference would be to squash these into a single commit, if others agree that we don't have to worry about length limits. At any rate, I'm quite impressed at the number of lines of code I was able to remove in order to fix a bug! Eric Blake (2): nwfilter: avoid failure with noexec /tmp nwfilter: simplify execution of ebiptables scripts src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++-- 1 files changed, 10 insertions(+), 124 deletions(-) This series does not solve a more fundamental issue that has been on my mind - are we still sure that the best design for manipulating network filters involves the creation of a series of shell scripting commands, where we have to worry about proper quoting and so forth? Is it possible to refactor this code to make more direct use of virCommand for every call to iptables and friends (that is, doing the glue logic in C rather than using C to generate shell scripting commands where the glue logic is in generated sh)? Or perhaps to even refactor things into a well-defined file format that we can feed to a helper executable, which would allow finer-grained SELinux labelling (by isolating the direct execution of iptables into a well-defined helper executable, then SELinux can enforce that libvirtd cannot alter the host firewall except by going through the helper executable, which has been audited to make only known sets of iptables calls based on well-formed input)? -- 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 0/2] fix nwfilter when /tmp is mounted noexec
On Wed, Nov 09, 2011 at 11:01:58AM -0700, Eric Blake wrote: This series does not solve a more fundamental issue that has been on my mind - are we still sure that the best design for manipulating network filters involves the creation of a series of shell scripting commands, where we have to worry about proper quoting and so forth? Is it possible to refactor this code to make more direct use of virCommand for every call to iptables and friends (that is, doing the glue logic in C rather than using C to generate shell scripting commands where the glue logic is in generated sh)? Or perhaps to even refactor things into a well-defined file format that we can feed to a helper executable, which would allow finer-grained SELinux labelling (by isolating the direct execution of iptables into a well-defined helper executable, then SELinux can enforce that libvirtd cannot alter the host firewall except by going through the helper executable, which has been audited to make only known sets of iptables calls based on well-formed input)? I think virCommand is a little too low level to do what nwfilter wants here, but we can definitely build a higher level API around it to replace the shell scripting. I has been a while since I looked at the generated scripts, IIUC, the shell scripts generated are basically defining groups of iptables commands to make interesting changes, and periodically a rollback command which is run if something fails. So I was thinking you could design a libvirt level API that would be used something like: virCommandScriptPtr script = virCommandScriptNew(); virCommandScriptBeginGroup(script); virCommandScriptAddCommandArgList(script, /some/command, args.); virCommandScriptAddRollbackArgList(script, /some/command, args...); virCommandScriptBeginGroup(script); virCommandScriptAddCommandArgList(script, /some/command, args.); virCommandScriptAddCommandArgList(script, /some/command, args.); virCommandScriptAddCommandArgList(script, /some/command, args.); ...repeat... virCommandScriptAddRollbackArgList(script, /some/command, args...); virCommandScriptRun(); If one of the commands following the 'BeginGroup' failed, then it would execute the Rollback command for that group, and for every preceeding group. Personally I would dearly love to kill off this use of shell scripting in libvirt, because I fear it is only a matter of time before we introduce some flaw wrt escaping. In general I'd like to be able to say that libvirt *never* runs shell commands, always uses execve() directly. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] nwfilter: simplify execution of ebiptables scripts
On 11/09/2011 12:46 PM, Eric Blake wrote: It's not worth even worrying about a temporary file, unless we ever expect the script to exceed maximum command-line argument length limits. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI): Run the commands as an argument to /bin/sh, rather than worrying about a temporary file. (ebiptablesWriteToTempFile): Delete unused function. --- src/nwfilter/nwfilter_ebiptables_driver.c | 88 +--- 1 files changed, 4 insertions(+), 84 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index c9c194c..aacbd02 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2476,65 +2476,6 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, /** - * ebiptablesWriteToTempFile: - * @string : the string to write into the file - * - * Returns the tempory filename where the string was written into, - * NULL in case of error with the error reported. - * - * Write the string into a temporary file and return the name of - * the temporary file. The file can then be read as a /bin/sh script. - * No '#!/bin/sh' header is needed, since the file will be read and not - * directly executed. - */ -static char * -ebiptablesWriteToTempFile(const char *string) { -char filename[] = LOCALSTATEDIR /run/libvirt/nwfilt-XX; -size_t len; -char *filnam; -size_t written; - -int fd = mkstemp(filename); - -if (fd 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot create temporary file)); -goto err_exit; -} - -len = strlen(string); -written = safewrite(fd, string, len); -if (written != len) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot write string to file)); -goto err_exit; -} - -if (VIR_CLOSE(fd) 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot write string to file)); -goto err_exit; -} - -filnam = strdup(filename); -if (!filnam) { -virReportOOMError(); -goto err_exit; -} - -return filnam; - -err_exit: -VIR_FORCE_CLOSE(fd); -unlink(filename); -return NULL; -} - - -/** * ebiptablesExecCLI: * @buf : pointer to virBuffer containing the string with the commands to *execute. @@ -2546,36 +2487,20 @@ err_exit: * script. * * Execute a sequence of commands (held in the given buffer) as a /bin/sh - * script and return the status of the execution. + * script and return the status of the execution in *status (if status is + * NULL, then the script must exit with status 0). */ static int ebiptablesExecCLI(virBufferPtr buf, int *status) { -char *cmds; -char *filename; int rc = -1; virCommandPtr cmd; -if (virBufferError(buf)) { -virReportOOMError(); -virBufferFreeAndReset(buf); -return -1; -} - *status = 0; Here I had to insert: if (!virBufferUse(buf)) return 0; -cmds = virBufferContentAndReset(buf); -VIR_DEBUG(%s, NULLSTR(cmds)); -if (!cmds) -return 0; - -filename = ebiptablesWriteToTempFile(cmds); -if (!filename) -goto cleanup; - -cmd = virCommandNew(/bin/sh); -virCommandAddArg(cmd, filename); +cmd = virCommandNewArgList(/bin/sh, -c, NULL); +virCommandAddArgBuffer(cmd, buf); virMutexLock(execCLIMutex); @@ -2583,11 +2508,6 @@ ebiptablesExecCLI(virBufferPtr buf, virMutexUnlock(execCLIMutex); -unlink(filename); -VIR_FREE(filename); - -cleanup: -VIR_FREE(cmds); virCommandFree(cmd); return rc; ACK with above nit fixed so it still works. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] nwfilter: avoid failure with noexec /tmp
On 11/09/2011 12:46 PM, Eric Blake wrote: If /tmp is mounted with the noexec flag (common on security-conscious systems), then nwfilter will fail to initialize, because we cannot run any temporary script via virRun(/tmp/script); but we _can_ use /bin/sh /tmp/script. For that matter, using /tmp risks collisions with other unrelated programs; we already have /var/run/libvirt as a dedicated temporary directory for use by libvirt. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Use internal directory, not /tmp; drop attempts to make script executable; and detect close error. (ebiptablesExecCLI): Switch to virCommand, and invoke the shell to read the script, rather than requiring an executable script. --- src/nwfilter/nwfilter_ebiptables_driver.c | 76 - 1 files changed, 21 insertions(+), 55 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index f87cfa1..c9c194c 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -40,6 +40,7 @@ #include nwfilter_ebiptables_driver.h #include virfile.h #include command.h +#include configmake.h #define VIR_FROM_THIS VIR_FROM_NWFILTER @@ -2482,29 +2483,17 @@ ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, * NULL in case of error with the error reported. * * Write the string into a temporary file and return the name of - * the temporary file. The string is assumed to contain executable - * commands. A line '#!/bin/sh' will automatically be written - * as the first line in the file. The permissions of the file are - * set so that the file can be run as an executable script. + * the temporary file. The file can then be read as a /bin/sh script. + * No '#!/bin/sh' header is needed, since the file will be read and not + * directly executed. */ static char * ebiptablesWriteToTempFile(const char *string) { -char filename[] = /tmp/virtdXX; -int len; +char filename[] = LOCALSTATEDIR /run/libvirt/nwfilt-XX; +size_t len; char *filnam; -virBuffer buf = VIR_BUFFER_INITIALIZER; -char *header; size_t written; -virBufferAddLit(buf, #!/bin/sh\n); - -if (virBufferError(buf)) { -virBufferFreeAndReset(buf); -virReportOOMError(); -return NULL; -} -header = virBufferContentAndReset(buf); - int fd = mkstemp(filename); if (fd 0) { @@ -2514,15 +2503,8 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -if (fchmod(fd, S_IXUSR| S_IRUSR | S_IWUSR) 0) { -virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - %s, - _(cannot change permissions on temp. file)); -goto err_exit; -} - -len = strlen(header); -written = safewrite(fd, header, len); +len = strlen(string); +written = safewrite(fd, string, len); if (written != len) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s, @@ -2530,9 +2512,7 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -len = strlen(string); -written = safewrite(fd, string, len); -if (written != len) { +if (VIR_CLOSE(fd) 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot write string to file)); @@ -2545,12 +2525,9 @@ ebiptablesWriteToTempFile(const char *string) { goto err_exit; } -VIR_FREE(header); -VIR_FORCE_CLOSE(fd); return filnam; err_exit: -VIR_FREE(header); VIR_FORCE_CLOSE(fd); unlink(filename); return NULL; @@ -2564,7 +2541,7 @@ err_exit: * @status: Pointer to an integer for returning the WEXITSTATUS of the *commands executed via the script the was run. * - * Returns 0 in case of success, != 0 in case of an error. The returned + * Returns 0 in case of success, 0 in case of an error. The returned * value is NOT the result of running the commands inside the shell * script. * @@ -2577,53 +2554,42 @@ ebiptablesExecCLI(virBufferPtr buf, { char *cmds; char *filename; -int rc; -const char *argv[] = {NULL, NULL}; +int rc = -1; +virCommandPtr cmd; if (virBufferError(buf)) { virReportOOMError(); virBufferFreeAndReset(buf); -return 1; +return -1; } *status = 0; cmds = virBufferContentAndReset(buf); - VIR_DEBUG(%s, NULLSTR(cmds)); - if (!cmds) return 0; filename = ebiptablesWriteToTempFile(cmds); -VIR_FREE(cmds); - if (!filename) -return 1; +goto cleanup; -argv[0] = filename; +cmd = virCommandNew(/bin/sh); +virCommandAddArg(cmd, filename); virMutexLock(execCLIMutex); -rc = virRun(argv, status); +rc =
Re: [libvirt] [PATCH 0/2] fix nwfilter when /tmp is mounted noexec
On 11/09/2011 01:01 PM, Eric Blake wrote: On 11/09/2011 10:46 AM, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=752254 points out that libvirt cannot support nwfilter on a system with /tmp mounted noexec (which is a very common setup in security-conscious setups), all because we were trying to directly invoke a temporary script instead of invoking a shell to read the script. I've split this patch into 2 parts, on the off-chance that patch 2 would run afoul of command line length limits (if the total size of the generated nwfilter commands could possibly cause E2BIG, then we have to go through a temporary file). But my recollection is that modern Linux kernels support unlimited command-line length (that is, ARG_MAX is not a concern on Linux), and that nwfilter_ebiptables_driver only compiles on Linux, so my preference would be to squash these into a single commit, if others agree that we don't have to worry about length limits. At any rate, I'm quite impressed at the number of lines of code I was able to remove in order to fix a bug! Eric Blake (2): nwfilter: avoid failure with noexec /tmp nwfilter: simplify execution of ebiptables scripts src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++-- 1 files changed, 10 insertions(+), 124 deletions(-) This series does not solve a more fundamental issue that has been on my mind - are we still sure that the best design for manipulating network filters involves the creation of a series of shell scripting commands, where we have to worry about proper quoting and so forth? Is it possible to refactor this code to make more direct use of We should at least have the quoting under control... virCommand for every call to iptables and friends (that is, doing the glue logic in C rather than using C to generate shell scripting commands where the glue logic is in generated sh)? Or perhaps to even refactor things into a well-defined file format that we can feed to a helper executable, which would allow finer-grained SELinux labelling I am sure the refactoring would be possible, but wouldn't it be more efficient if this type of post-processing was done in a batch with as many commands collected as possible ? So that would mean keeping the general code but finding a well-defined file format for probably ebtables and iptables rules. (by isolating the direct execution of iptables into a well-defined helper executable, then SELinux can enforce that libvirtd cannot alter the host firewall except by going through the helper executable, which has been audited to make only known sets of iptables calls based on well-formed input)? The Network filter subsystem only touches the FORWARD chain of iptables and none of the INPUT chains or its 'descendants'. Of course one can double -check that in another process. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] Enable the blkdeviotune command in virsh
Support virsh command blkdeviotune. Can set or query a block disk I/O throttle setting. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- tools/virsh.c | 146 +++ tools/virsh.pod | 23 + 2 files changed, 169 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 83dc3c7..df2b399 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6023,6 +6023,151 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) return true; } +/* + * blkdeviotune command + */ +static const vshCmdInfo info_blkdeviotune[] = { +{help, N_(Set or query a block disk I/O throttle setting.)}, +{desc, N_(Set or query a block disk I/O throttle setting.\n \ +To query the block disk I/O throttle setting use the following \ + command: \n\n \ +virsh # blkdeviotune domain device)}, +{NULL, NULL} +}; + +static const vshCmdOptDef opts_blkdeviotune[] = { +{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)}, +{device, VSH_OT_DATA, VSH_OFLAG_REQ, N_(block device)}, +{total_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total throughput limit in bytes per second/s)}, +{read_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read throughput limit in bytes per second/s)}, +{write_bytes_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write throughput limit in bytes per second/s)}, +{total_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(total I/O operations limit per second/s)}, +{read_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(read I/O operations limit per second/s)}, +{write_iops_sec, VSH_OT_INT, VSH_OFLAG_NONE, N_(write I/O operations limit per second/s)}, +{config, VSH_OT_BOOL, 0, N_(affect next boot)}, +{live, VSH_OT_BOOL, 0, N_(affect running domain)}, +{current, VSH_OT_BOOL, 0, N_(affect current domain)}, +{NULL, 0, 0, NULL} +}; + +static bool +cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +const char *name, *disk; +virDomainBlockIoTuneInfo info; +virDomainBlockIoTuneInfo reply; +unsigned int flags = 0; +int ret = -1; +int rv = 0; +int set = 0; +int current = vshCommandOptBool(cmd, current); +int config = vshCommandOptBool(cmd, config); +int live = vshCommandOptBool(cmd, live); + +if (current) { +if (live || config) { +vshError(ctl, %s, _(--current must be specified exclusively)); +return false; +} +flags = VIR_DOMAIN_AFFECT_CURRENT; +} else { +if (config) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; +} + +memset(info, 0, sizeof(info)); + +if (!vshConnectionUsability(ctl, ctl-conn)) +goto out; + +if (!(dom = vshCommandOptDomain(ctl, cmd, name))) +goto out; + +if (vshCommandOptString(cmd, device, disk) 0) +goto out; + +if ((rv = vshCommandOptULongLong(cmd, total_bytes_sec, info.total_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_bytes_sec, info.read_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_bytes_sec, info.write_bytes_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, total_iops_sec, info.total_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, read_iops_sec, info.read_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if ((rv = vshCommandOptULongLong(cmd, write_iops_sec, info.write_iops_sec)) 0) { +vshError(ctl, %s, + _(Unable to parse integer parameter)); +goto out; +} else if (rv 0) { +set++; +} + +if (!set) { + +ret = virDomainGetBlockIoTune(dom, disk, reply, flags); + +if (ret != 0) +goto out; + +vshPrint(ctl, %-15s %llu\n, _(total_bytes_sec:), reply.total_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(read_bytes_sec:), reply.read_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(write_bytes_sec:), reply.write_bytes_sec); +vshPrint(ctl, %-15s %llu\n, _(total_iops_sec:), reply.total_iops_sec); +vshPrint(ctl, %-15s %llu\n, _(read_iops_sec:),
[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c| 115 ++ src/libvirt_public.syms |2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { +unsigned long long total_bytes_sec; +unsigned long long read_bytes_sec; +unsigned long long write_bytes_sec; +unsigned long long total_iops_sec; +unsigned long long read_iops_sec; +unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; + +int +virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr info, +unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr reply, +unsigned int flags); + /* * NUMA support diff --git a/src/driver.h b/src/driver.h index 4c14aaa..9628ad7 100644 --- a/src/driver.h +++ b/src/driver.h @@ -741,6 +741,23 @@ typedef int unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int +(*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags); + +typedef int +(*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr reply, + unsigned int flags); + + /** * _virDriver: * @@ -899,6 +916,8 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; +virDrvDomainSetBlockIoTune domainSetBlockIoTune; +virDrvDomainGetBlockIoTune domainGetBlockIoTune; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index b0d1e01..79ac84d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17083,3 +17083,118 @@ error: virDispatchError(dom-conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @info: Specify block I/O limits in bytes + * @flags: An OR'ed set of virDomainModificationImpact + * + * This function is mainly to enable Block I/O throttling function in libvirt. + * It is used to change the block I/O throttling setting for specified domain. + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainSetBlockIoTune(virDomainPtr dom, +const char *disk, +virDomainBlockIoTuneInfoPtr info, +unsigned int flags) +{ +virConnectPtr conn; + +VIR_DOMAIN_DEBUG(dom, disk=%p, info=%p, flags=%x, + disk, info, flags); + +virResetLastError(); + +if (!VIR_IS_CONNECTED_DOMAIN (dom)) { +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); +virDispatchError(NULL); +return -1; +} +conn = dom-conn; + +if (dom-conn-flags VIR_CONNECT_RO) { +virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); +goto error; +} + +if (!disk) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +if (!info) { +virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); +goto error; +} + +if (conn-driver-domainSetBlockIoTune) { +int ret; +ret = conn-driver-domainSetBlockIoTune(dom, disk, info, flags); +if (ret 0) +goto error; +return ret; +} + +virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(dom-conn); +return -1; +} + +/** + * virDomainGetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @reply: Specify block I/O info in bytes + * @flags: An OR'ed set of virDomainModificationImpact + * + * This function is
[libvirt] [PATCH 3/8] Implement virDomain{Set, Get}BlockIoTune for the qemu driver
This patch implement the blk io throttle setting and getting support to qemu driver. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c | 216 ++ src/qemu/qemu_monitor.c | 36 +++ src/qemu/qemu_monitor.h | 10 ++ src/qemu/qemu_monitor_json.c | 191 + src/qemu/qemu_monitor_json.h | 10 ++ src/qemu/qemu_monitor_text.c | 152 + src/qemu/qemu_monitor_text.h | 10 ++ 7 files changed, 625 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index db2ac0d..7ca6719 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10759,6 +10759,220 @@ cleanup: return ret; } +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ +struct qemud_driver *driver = dom-conn-privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +virDomainDefPtr persistentDef = NULL; +char uuidstr[VIR_UUID_STRING_BUFLEN]; +const char *device = NULL; +int ret = -1; +bool isActive; + +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + +qemuDriverLock(driver); +virUUIDFormat(dom-uuid, uuidstr); +vm = virDomainFindByUUID(driver-domains, dom-uuid); +if (!vm) { +qemuReportError(VIR_ERR_NO_DOMAIN, +_(no domain with matching uuid '%s'), uuidstr); +goto cleanup; +} + +device = qemuDiskPathToAlias(vm, disk); +if (!device) { +goto cleanup; +} + +if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0) +goto cleanup; + +isActive = virDomainObjIsActive(vm); + +if (flags == VIR_DOMAIN_AFFECT_CURRENT) { +if (isActive) +flags = VIR_DOMAIN_AFFECT_LIVE; +else +flags = VIR_DOMAIN_AFFECT_CONFIG; +} + +if ((info-total_bytes_sec info-read_bytes_sec) || +(info-total_bytes_sec info-write_bytes_sec)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(bps and bps_rd/bps_wr cannot be used at the same time)); +goto endjob; +} + +if ((info-total_iops_sec info-read_iops_sec) || +(info-total_iops_sec info-write_iops_sec)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(iops and iops_rd/iops_wr cannot be used at the same time)); +goto endjob; +} + +if (!isActive (flags VIR_DOMAIN_AFFECT_LIVE)) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(domain is not running)); +goto endjob; +} + + if (flags VIR_DOMAIN_AFFECT_CONFIG) { +if (!vm-persistent) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, +_(cannot change persistent config of a transient domain)); +goto endjob; +} +if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm))) +goto endjob; +} + +if (flags VIR_DOMAIN_AFFECT_LIVE) { +priv = vm-privateData; +qemuDomainObjEnterMonitorWithDriver(driver, vm); +ret = qemuMonitorSetBlockIoThrottle(priv-mon, device, info, 1); +qemuDomainObjExitMonitorWithDriver(driver, vm); +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +sa_assert(persistentDef); +int i = virDomainDiskIndexByName(vm-def, disk, true); +if (i 0) +goto endjob; +persistentDef-disks[i]-blkdeviotune.total_bytes_sec = info-total_bytes_sec; +persistentDef-disks[i]-blkdeviotune.read_bytes_sec = info-read_bytes_sec; +persistentDef-disks[i]-blkdeviotune.write_bytes_sec = info-write_bytes_sec; +persistentDef-disks[i]-blkdeviotune.total_iops_sec = info-total_iops_sec; +persistentDef-disks[i]-blkdeviotune.read_iops_sec = info-read_iops_sec; +persistentDef-disks[i]-blkdeviotune.write_iops_sec = info-write_iops_sec; +persistentDef-disks[i]-blkdeviotune.mark = 1; +} + +if (flags VIR_DOMAIN_AFFECT_CONFIG) { +ret = virDomainSaveConfig(driver-configDir, persistentDef); +if (ret 0) { +qemuReportError(VIR_ERR_OPERATION_INVALID, %s, + _(Write to config file failed)); +goto endjob; +} +} + +endjob: +if (qemuDomainObjEndJob(driver, vm) == 0) +vm = NULL; + +cleanup: +VIR_FREE(device); +if (vm) +virDomainObjUnlock(vm); +qemuDriverUnlock(driver); +return ret; +} + +static int +qemuDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virDomainBlockIoTuneInfoPtr reply, +
[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- src/conf/domain_conf.c | 101 +- src/conf/domain_conf.h | 12 ++ src/qemu/qemu_command.c | 33 +++ src/util/xml.h |2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..564914e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child-next; } +} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) { +if (virXPathULongLong(string(./devices/disk/iotune/total_bytes_sec), + ctxt, def-blkdeviotune.total_bytes_sec) 0) { +def-blkdeviotune.total_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_bytes_sec), + ctxt, def-blkdeviotune.read_bytes_sec) 0) { +def-blkdeviotune.read_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_bytes_sec), + ctxt, def-blkdeviotune.write_bytes_sec) 0) { +def-blkdeviotune.write_bytes_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + + if (virXPathULongLong(string(./devices/disk/iotune/total_iops_sec), + ctxt, def-blkdeviotune.total_iops_sec) 0) { +def-blkdeviotune.total_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/read_iops_sec), + ctxt, def-blkdeviotune.read_iops_sec) 0) { +def-blkdeviotune.read_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if (virXPathULongLong(string(./devices/disk/iotune/write_iops_sec), + ctxt, def-blkdeviotune.write_iops_sec) 0) { +def-blkdeviotune.write_iops_sec = 0; +} else { +def-blkdeviotune.mark = 1; +} + +if ((def-blkdeviotune.total_bytes_sec def-blkdeviotune.read_bytes_sec) +|| (def-blkdeviotune.total_bytes_sec def-blkdeviotune.write_bytes_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(bps and bps_rd/bps_wr cannot be set at the same time)); +goto error; +} + +if ((def-blkdeviotune.total_iops_sec def-blkdeviotune.read_iops_sec) +|| (def-blkdeviotune.total_iops_sec def-blkdeviotune.write_iops_sec)) { +virDomainReportError(VIR_ERR_XML_ERROR, + _(iops and iops_rd/iops_wr cannot be set at the same time)); +goto error; +} } else if (xmlStrEqual(cur-name, BAD_CAST readonly)) { def-readonly = 1; } else if (xmlStrEqual(cur-name, BAD_CAST shareable)) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node-name, BAD_CAST disk)) { dev-type = VIR_DOMAIN_DEVICE_DISK; if (!(dev-data.disk = virDomainDiskDefParseXML(caps, node, -NULL, flags))) +NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node-name, BAD_CAST lease)) { dev-type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, -flags); +
[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
Support Block I/O Throttle setting and query to remote driver. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- daemon/remote.c | 87 ++ src/remote/remote_driver.c | 80 ++ src/remote/remote_protocol.x | 38 ++- src/remote_protocol-structs | 33 4 files changed, 237 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index bd0c3e3..070ca65 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1850,6 +1850,93 @@ cleanup: return rv; } +static int +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_set_block_io_throttle_args *args) +{ +virDomainPtr dom = NULL; +virDomainBlockIoTuneInfo tmp; +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 (args) { +tmp.total_bytes_sec = args-bps; +tmp.read_bytes_sec = args-bps_rd; +tmp.write_bytes_sec = args-bps_wr; +tmp.total_iops_sec = args-iops; +tmp.read_iops_sec = args-iops_rd; +tmp.write_iops_sec = args-iops_wr; +} + +rv = virDomainSetBlockIoTune(dom, args-disk, tmp, args-flags); + +if (rv 0) +goto cleanup; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} + +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ +virDomainPtr dom = NULL; +virDomainBlockIoTuneInfo reply; +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; + +rv = virDomainGetBlockIoTune(dom, args-disk, reply, args-flags); + +if (rv 0) { +ret-found = 0; +goto cleanup; +} + +ret-bps = reply.total_bytes_sec; +ret-bps_rd = reply.read_bytes_sec; +ret-bps_wr = reply.write_bytes_sec; +ret-iops= reply.total_iops_sec; +ret-iops_rd = reply.read_iops_sec; +ret-iops_wr = reply.write_iops_sec; +ret-found = 1; + +cleanup: +if (rv 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} /*-*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f3b8ad5..0900231 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2154,6 +2154,84 @@ done: return rv; } +static int remoteDomainSetBlockIoTune(virDomainPtr domain, + const char *disk, + virDomainBlockIoTuneInfoPtr info, + unsigned int flags) +{ +int rv = -1; +remote_domain_set_block_io_throttle_args args; +struct private_data *priv = domain-conn-privateData; + +remoteDriverLock(priv); + +memset(args, 0, sizeof(args)); + +make_nonnull_domain(args.dom, domain); +args.disk = (char *)disk; +args.bps = info-total_bytes_sec; +args.bps_rd = info-read_bytes_sec; +args.bps_wr = info-write_bytes_sec; +args.iops = info-total_iops_sec; +args.iops_rd = info-read_iops_sec; +args.iops_wr = info-write_iops_sec; +args.flags = flags; + +if (call(domain-conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_set_block_io_throttle_args, + (char *) args, + (xdrproc_t) xdr_void, + (char *) NULL) == -1) { +goto done; +} +rv = 0; + +done: +remoteDriverUnlock(priv); +return rv; +} + +static int remoteDomainGetBlockIoTune(virDomainPtr domain, + const char *disk, +
[libvirt] [RFC PATCH 0/8 v3] Summary on block IO throttle
Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotune domain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotune domain device [--total_bytes_sec number] [--read_bytes_sec number] [--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec number] [--write_iops_sec number] [--config] [--live] [--current] daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py |2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c| 101 src/conf/domain_conf.h| 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms |2 src/qemu/qemu_command.c | 33 ++ src/qemu/qemu_driver.c| 217 ++ src/qemu/qemu_monitor.c | 36 ++ src/qemu/qemu_monitor.h | 10 src/qemu/qemu_monitor_json.c | 191 +++ src/qemu/qemu_monitor_json.h | 10 src/qemu/qemu_monitor_text.c | 152 src/qemu/qemu_monitor_text.h | 10 src/remote/remote_driver.c| 81 ++ src/remote/remote_protocol.x | 39 +++ src/remote_protocol-structs | 34 ++ src/util/xml.h|3 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune.args |4
[libvirt] [PATCH 7/8] Support virDomain{Set, Get}BlockIoThrottle in the python API
Python support for both set and get block I/O throttle. Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Lei Li li...@linux.vnet.ibm.com --- python/generator.py |2 + python/libvirt-override-api.xml | 16 +++ python/libvirt-override.c | 85 +++ 3 files changed, 103 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index 71afdb7..88c52b9 100755 --- a/python/generator.py +++ b/python/generator.py @@ -414,6 +414,8 @@ skip_impl = ( 'virDomainGetBlockJobInfo', 'virDomainMigrateGetMaxSpeed', 'virDomainBlockStatsFlags', +'virDomainSetBlockIoTune', +'virDomainGetBlockIoTune', ) qemu_skip_impl = ( diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index ef02f34..e8d9138 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -375,5 +375,21 @@ arg name='flags' type='unsigned int' info='flags, currently unused, pass 0.'/ return type='unsigned long' info='current max migration speed, or None in case of error'/ /function +function name='virDomainSetBlockIoTune' file='python' + infoChange the I/O throttle for a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='info' type='virDomainBlockIoTuneInfoPtr' info='io throttle limits'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='virDomainSetBlockIoTuneInfo' info='0 in case of operation has started, -1 in case of failure'/ +/function +function name='virDomainGetBlockIoTune' file='python' + infoGet the I/O throttle a block device/info + arg name='dom' type='virDomainPtr' info='pointer to the domain'/ + arg name='disk' type='const char *' info='disk name'/ + arg name='reply' type='virDomainBlockIoTuneInfoPtr' info='io throttle info'/ + arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainModificationImpact'/ + return type='virDomainGetBlockIoTuneInfo' info='A dictionary containing block I/O limits information.'/ +/function /symbols /api diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1759bae..d4b023a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3195,6 +3195,89 @@ LIBVIRT_END_ALLOW_THREADS; return ret; } +static PyObject * +libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain, *pyinfo; +const char *disk; +unsigned int flags; +virDomainBlockIoTuneInfo info; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Ozi:virDomainSetBlockIoTune, + pyobj_domain, disk, pyinfo, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +info.total_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, +libvirt_constcharPtrWrap(bps))); +info.read_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(bps_rd))); +info.write_bytes_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, +libvirt_constcharPtrWrap(bps_wr))); +info.total_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops))); +info.read_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops_rd))); +info.write_iops_sec = (unsigned long long)PyLong_AsLongLong(PyDict_GetItem(pyinfo, + libvirt_constcharPtrWrap(iops_wr))); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainSetBlockIoTune(domain, disk, info, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) { +return VIR_PY_NONE; +} + +return VIR_PY_INT_SUCCESS; +} + +static PyObject * +libvirt_virDomainGetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, +PyObject *args) +{ +virDomainPtr domain; +PyObject *pyobj_domain; +const char *disk; +unsigned int flags; +virDomainBlockIoTuneInfo reply; +int c_ret; +PyObject *ret; + +if (!PyArg_ParseTuple(args, (char *)Oi:virDomainGetBlockIoTune, + pyobj_domain, disk, flags)) +return(NULL); +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; +c_ret = virDomainGetBlockIoTune(domain, disk, reply, flags); +LIBVIRT_END_ALLOW_THREADS; + +if (c_ret 0) +return VIR_PY_NONE; + +if ((ret = PyDict_New()) == NULL) +return VIR_PY_NONE; + +
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/09/2011 01:32 PM, Lei Li wrote: This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c| 115 ++ src/libvirt_public.syms |2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { +unsigned long long total_bytes_sec; +unsigned long long read_bytes_sec; +unsigned long long write_bytes_sec; +unsigned long long total_iops_sec; +unsigned long long read_iops_sec; +unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure. -- 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
[libvirt] [libvirt-glib 2/3] Fix creation of empty XML document
It's not enough to assign xmlDocPtr::children to set the document root, xmlDocSetRootElement must be used instead. --- libvirt-gconfig/libvirt-gconfig-object.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index adbd955..fbdbedd 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -378,11 +378,13 @@ GVirConfigObject *gvir_config_object_new(GType type, const char *schema) { xmlDocPtr doc; +xmlNodePtr node; doc = xmlNewDoc((xmlChar *)1.0); -doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); +node = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); +xmlDocSetRootElement(doc, node); return GVIR_CONFIG_OBJECT(g_object_new(type, - node, doc-children, + node, node, schema, schema, NULL)); } -- 1.7.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib 1/3] Add gvir_config_object_new{_from_xml} helpers
This factors the code that is repeated in the _new methods of all classes deriving from GVirObject --- libvirt-gconfig/libvirt-gconfig-domain.c | 26 ++-- libvirt-gconfig/libvirt-gconfig-object.c | 31 ++ libvirt-gconfig/libvirt-gconfig-object.h |8 +++ libvirt-gconfig/libvirt-gconfig.sym |1 + 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 837287c..3290389 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -147,27 +147,23 @@ static void gvir_config_domain_init(GVirConfigDomain *conn) GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml, GError **error) { -xmlNodePtr node; +GVirConfigObject *object; -node = gvir_config_xml_parse(xml, domain, error); -if ((error != NULL) (*error != NULL)) -return NULL; -return GVIR_CONFIG_DOMAIN(g_object_new(GVIR_TYPE_CONFIG_DOMAIN, - node, node, - schema, DATADIR /libvirt/schemas/domain.rng, - NULL)); +object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN, + domain, + DATADIR /libvirt/schemas/domain.rng, + xml, error); +return GVIR_CONFIG_DOMAIN(object); } GVirConfigDomain *gvir_config_domain_new(void) { -xmlDocPtr doc; +GVirConfigObject *object; -doc = xmlNewDoc((xmlChar *)1.0); -doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)domain, NULL); -return GVIR_CONFIG_DOMAIN(g_object_new(GVIR_TYPE_CONFIG_DOMAIN, - node, doc-children, - schema, DATADIR /libvirt/schemas/domain.rng, - NULL)); +object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN, +domain, +DATADIR /libvirt/schemas/domain.rng); +return GVIR_CONFIG_DOMAIN(object); } char *gvir_config_domain_get_name(GVirConfigDomain *domain) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index e59cbcd..adbd955 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -355,3 +355,34 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, gvir_config_object_set_node_content(object, node_name, str); g_free(str); } + +GVirConfigObject *gvir_config_object_new_from_xml(GType type, + const char *root_name, + const char *schema, + const gchar *xml, + GError **error) +{ +xmlNodePtr node; + +node = gvir_config_xml_parse(xml, root_name, error); +if ((error != NULL) (*error != NULL)) +return NULL; +return GVIR_CONFIG_OBJECT(g_object_new(type, + node, node, + schema, schema, + NULL)); +} + +GVirConfigObject *gvir_config_object_new(GType type, + const char *root_name, + const char *schema) +{ +xmlDocPtr doc; + +doc = xmlNewDoc((xmlChar *)1.0); +doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); +return GVIR_CONFIG_OBJECT(g_object_new(type, + node, doc-children, + schema, schema, + NULL)); +} diff --git a/libvirt-gconfig/libvirt-gconfig-object.h b/libvirt-gconfig/libvirt-gconfig-object.h index c203798..52e4525 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.h +++ b/libvirt-gconfig/libvirt-gconfig-object.h @@ -59,6 +59,14 @@ struct _GVirConfigObjectClass GType gvir_config_object_get_type(void); +GVirConfigObject *gvir_config_object_new(GType type, + const char *root_name, + const char *schema); +GVirConfigObject *gvir_config_object_new_from_xml(GType type, + const char *root_name, + const char *schema, + const gchar *xml, + GError **error); void gvir_config_object_validate(GVirConfigObject *config,
[libvirt] [libvirt-glib 3/3] Use new creation helpers everywhere
Use the new gvir_config_object_new{_from_xml} in all classes inheriting from GVirConfigObject. This simplifies the code in the _new methods creating empty objects. This commit also adds _new_from_xml methods to all these objects. --- libvirt-gconfig/libvirt-gconfig-capabilities.c| 26 +++- libvirt-gconfig/libvirt-gconfig-capabilities.h|2 + libvirt-gconfig/libvirt-gconfig-domain-snapshot.c | 23 +- libvirt-gconfig/libvirt-gconfig-domain-snapshot.h |2 + libvirt-gconfig/libvirt-gconfig-interface.c | 25 ++- libvirt-gconfig/libvirt-gconfig-interface.h |2 + libvirt-gconfig/libvirt-gconfig-network-filter.c | 24 +- libvirt-gconfig/libvirt-gconfig-network-filter.h |2 + libvirt-gconfig/libvirt-gconfig-network.c | 24 +- libvirt-gconfig/libvirt-gconfig-network.h |2 + libvirt-gconfig/libvirt-gconfig-node-device.c | 25 +-- libvirt-gconfig/libvirt-gconfig-node-device.h |2 + libvirt-gconfig/libvirt-gconfig-secret.c | 24 +- libvirt-gconfig/libvirt-gconfig-secret.h |2 + libvirt-gconfig/libvirt-gconfig-storage-pool.c| 24 +- libvirt-gconfig/libvirt-gconfig-storage-pool.h|2 + libvirt-gconfig/libvirt-gconfig-storage-vol.c | 24 +- libvirt-gconfig/libvirt-gconfig-storage-vol.h |2 + libvirt-gconfig/libvirt-gconfig.sym |9 +++ 19 files changed, 182 insertions(+), 64 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.c b/libvirt-gconfig/libvirt-gconfig-capabilities.c index 8ed0e2e..2f79073 100644 --- a/libvirt-gconfig/libvirt-gconfig-capabilities.c +++ b/libvirt-gconfig/libvirt-gconfig-capabilities.c @@ -63,14 +63,26 @@ static void gvir_config_capabilities_init(GVirConfigCapabilities *conn) GVirConfigCapabilities *gvir_config_capabilities_new(void) { -xmlDocPtr doc; +GVirConfigObject *object; -doc = xmlNewDoc((xmlChar *)1.0); /* FIXME: what is the XML root of the capability node? I suspect it is * either 'guest' or 'host' */ -doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)capabilities, NULL); -return GVIR_CONFIG_CAPABILITIES(g_object_new(GVIR_TYPE_CONFIG_CAPABILITIES, - node, doc-children, - schema, DATADIR /libvirt/schemas/capability.rng, - NULL)); +object = gvir_config_object_new(GVIR_TYPE_CONFIG_CAPABILITIES, +capabilities, +DATADIR /libvirt/schemas/capability.rng); +return GVIR_CONFIG_CAPABILITIES(object); +} + +GVirConfigCapabilities *gvir_config_capabilities_new_from_xml(const gchar *xml, + GError **error) +{ +GVirConfigObject *object; + +/* FIXME: what is the XML root of the capability node? I suspect it is + * either 'guest' or 'host' */ +object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_CAPABILITIES, + capabilities, + DATADIR /libvirt/schemas/capability.rng, + xml, error); +return GVIR_CONFIG_CAPABILITIES(object); } diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.h b/libvirt-gconfig/libvirt-gconfig-capabilities.h index b5660df..7170683 100644 --- a/libvirt-gconfig/libvirt-gconfig-capabilities.h +++ b/libvirt-gconfig/libvirt-gconfig-capabilities.h @@ -60,6 +60,8 @@ struct _GVirConfigCapabilitiesClass GType gvir_config_capabilities_get_type(void); GVirConfigCapabilities *gvir_config_capabilities_new(void); +GVirConfigCapabilities *gvir_config_capabilities_new_from_xml(const gchar *xml, + GError **error); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c index 35f63ec..e7dc36e 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c @@ -63,13 +63,22 @@ static void gvir_config_domain_snapshot_init(GVirConfigDomainSnapshot *conn) GVirConfigDomainSnapshot *gvir_config_domain_snapshot_new(void) { -xmlDocPtr doc; +GVirConfigObject *object; -doc = xmlNewDoc((xmlChar *)1.0); -doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)domainsnapshot, NULL); +object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_SNAPSHOT, +domainsnapshot, +DATADIR /libvirt/schemas/domainsnapshot.rng); +return GVIR_CONFIG_DOMAIN_SNAPSHOT(object); +} + +GVirConfigDomainSnapshot *gvir_config_domain_snapshot_new_from_xml(const
Re: [libvirt] [libvirt-glib 2/3] Fix creation of empty XML document
ack On Wed, Nov 9, 2011 at 9:48 PM, Christophe Fergeau cferg...@redhat.com wrote: It's not enough to assign xmlDocPtr::children to set the document root, xmlDocSetRootElement must be used instead. --- libvirt-gconfig/libvirt-gconfig-object.c | 6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index adbd955..fbdbedd 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -378,11 +378,13 @@ GVirConfigObject *gvir_config_object_new(GType type, const char *schema) { xmlDocPtr doc; + xmlNodePtr node; doc = xmlNewDoc((xmlChar *)1.0); - doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); + node = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); + xmlDocSetRootElement(doc, node); return GVIR_CONFIG_OBJECT(g_object_new(type, - node, doc-children, + node, node, schema, schema, NULL)); } -- 1.7.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/3] Add gvir_config_object_new{_from_xml} helpers
ack On Wed, Nov 9, 2011 at 9:48 PM, Christophe Fergeau cferg...@redhat.com wrote: This factors the code that is repeated in the _new methods of all classes deriving from GVirObject --- libvirt-gconfig/libvirt-gconfig-domain.c | 26 ++-- libvirt-gconfig/libvirt-gconfig-object.c | 31 ++ libvirt-gconfig/libvirt-gconfig-object.h | 8 +++ libvirt-gconfig/libvirt-gconfig.sym | 1 + 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 837287c..3290389 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -147,27 +147,23 @@ static void gvir_config_domain_init(GVirConfigDomain *conn) GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml, GError **error) { - xmlNodePtr node; + GVirConfigObject *object; - node = gvir_config_xml_parse(xml, domain, error); - if ((error != NULL) (*error != NULL)) - return NULL; - return GVIR_CONFIG_DOMAIN(g_object_new(GVIR_TYPE_CONFIG_DOMAIN, - node, node, - schema, DATADIR /libvirt/schemas/domain.rng, - NULL)); + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN, + domain, + DATADIR /libvirt/schemas/domain.rng, + xml, error); + return GVIR_CONFIG_DOMAIN(object); } GVirConfigDomain *gvir_config_domain_new(void) { - xmlDocPtr doc; + GVirConfigObject *object; - doc = xmlNewDoc((xmlChar *)1.0); - doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)domain, NULL); - return GVIR_CONFIG_DOMAIN(g_object_new(GVIR_TYPE_CONFIG_DOMAIN, - node, doc-children, - schema, DATADIR /libvirt/schemas/domain.rng, - NULL)); + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN, + domain, + DATADIR /libvirt/schemas/domain.rng); + return GVIR_CONFIG_DOMAIN(object); } char *gvir_config_domain_get_name(GVirConfigDomain *domain) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index e59cbcd..adbd955 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -355,3 +355,34 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, gvir_config_object_set_node_content(object, node_name, str); g_free(str); } + +GVirConfigObject *gvir_config_object_new_from_xml(GType type, + const char *root_name, + const char *schema, + const gchar *xml, + GError **error) +{ + xmlNodePtr node; + + node = gvir_config_xml_parse(xml, root_name, error); + if ((error != NULL) (*error != NULL)) + return NULL; + return GVIR_CONFIG_OBJECT(g_object_new(type, + node, node, + schema, schema, + NULL)); +} + +GVirConfigObject *gvir_config_object_new(GType type, + const char *root_name, + const char *schema) +{ + xmlDocPtr doc; + + doc = xmlNewDoc((xmlChar *)1.0); + doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); + return GVIR_CONFIG_OBJECT(g_object_new(type, + node, doc-children, + schema, schema, + NULL)); +} diff --git a/libvirt-gconfig/libvirt-gconfig-object.h b/libvirt-gconfig/libvirt-gconfig-object.h index c203798..52e4525 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.h +++ b/libvirt-gconfig/libvirt-gconfig-object.h @@ -59,6 +59,14 @@ struct _GVirConfigObjectClass GType gvir_config_object_get_type(void); +GVirConfigObject *gvir_config_object_new(GType type, + const char *root_name, + const char *schema); +GVirConfigObject *gvir_config_object_new_from_xml(GType type, + const char *root_name, + const char *schema, +
Re: [libvirt] [libvirt-glib 3/3] Use new creation helpers everywhere
ack On Wed, Nov 9, 2011 at 9:48 PM, Christophe Fergeau cferg...@redhat.com wrote: Use the new gvir_config_object_new{_from_xml} in all classes inheriting from GVirConfigObject. This simplifies the code in the _new methods creating empty objects. This commit also adds _new_from_xml methods to all these objects. --- libvirt-gconfig/libvirt-gconfig-capabilities.c | 26 +++- libvirt-gconfig/libvirt-gconfig-capabilities.h | 2 + libvirt-gconfig/libvirt-gconfig-domain-snapshot.c | 23 +- libvirt-gconfig/libvirt-gconfig-domain-snapshot.h | 2 + libvirt-gconfig/libvirt-gconfig-interface.c | 25 ++- libvirt-gconfig/libvirt-gconfig-interface.h | 2 + libvirt-gconfig/libvirt-gconfig-network-filter.c | 24 +- libvirt-gconfig/libvirt-gconfig-network-filter.h | 2 + libvirt-gconfig/libvirt-gconfig-network.c | 24 +- libvirt-gconfig/libvirt-gconfig-network.h | 2 + libvirt-gconfig/libvirt-gconfig-node-device.c | 25 +-- libvirt-gconfig/libvirt-gconfig-node-device.h | 2 + libvirt-gconfig/libvirt-gconfig-secret.c | 24 +- libvirt-gconfig/libvirt-gconfig-secret.h | 2 + libvirt-gconfig/libvirt-gconfig-storage-pool.c | 24 +- libvirt-gconfig/libvirt-gconfig-storage-pool.h | 2 + libvirt-gconfig/libvirt-gconfig-storage-vol.c | 24 +- libvirt-gconfig/libvirt-gconfig-storage-vol.h | 2 + libvirt-gconfig/libvirt-gconfig.sym | 9 +++ 19 files changed, 182 insertions(+), 64 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.c b/libvirt-gconfig/libvirt-gconfig-capabilities.c index 8ed0e2e..2f79073 100644 --- a/libvirt-gconfig/libvirt-gconfig-capabilities.c +++ b/libvirt-gconfig/libvirt-gconfig-capabilities.c @@ -63,14 +63,26 @@ static void gvir_config_capabilities_init(GVirConfigCapabilities *conn) GVirConfigCapabilities *gvir_config_capabilities_new(void) { - xmlDocPtr doc; + GVirConfigObject *object; - doc = xmlNewDoc((xmlChar *)1.0); /* FIXME: what is the XML root of the capability node? I suspect it is * either 'guest' or 'host' */ - doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)capabilities, NULL); - return GVIR_CONFIG_CAPABILITIES(g_object_new(GVIR_TYPE_CONFIG_CAPABILITIES, - node, doc-children, - schema, DATADIR /libvirt/schemas/capability.rng, - NULL)); + object = gvir_config_object_new(GVIR_TYPE_CONFIG_CAPABILITIES, + capabilities, + DATADIR /libvirt/schemas/capability.rng); + return GVIR_CONFIG_CAPABILITIES(object); +} + +GVirConfigCapabilities *gvir_config_capabilities_new_from_xml(const gchar *xml, + GError **error) +{ + GVirConfigObject *object; + + /* FIXME: what is the XML root of the capability node? I suspect it is + * either 'guest' or 'host' */ + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_CAPABILITIES, + capabilities, + DATADIR /libvirt/schemas/capability.rng, + xml, error); + return GVIR_CONFIG_CAPABILITIES(object); } diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities.h b/libvirt-gconfig/libvirt-gconfig-capabilities.h index b5660df..7170683 100644 --- a/libvirt-gconfig/libvirt-gconfig-capabilities.h +++ b/libvirt-gconfig/libvirt-gconfig-capabilities.h @@ -60,6 +60,8 @@ struct _GVirConfigCapabilitiesClass GType gvir_config_capabilities_get_type(void); GVirConfigCapabilities *gvir_config_capabilities_new(void); +GVirConfigCapabilities *gvir_config_capabilities_new_from_xml(const gchar *xml, + GError **error); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c index 35f63ec..e7dc36e 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-snapshot.c @@ -63,13 +63,22 @@ static void gvir_config_domain_snapshot_init(GVirConfigDomainSnapshot *conn) GVirConfigDomainSnapshot *gvir_config_domain_snapshot_new(void) { - xmlDocPtr doc; + GVirConfigObject *object; - doc = xmlNewDoc((xmlChar *)1.0); - doc-children = xmlNewDocNode(doc, NULL, (xmlChar *)domainsnapshot, NULL); + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_SNAPSHOT, + domainsnapshot, +
[libvirt] 0.9.7 virsh console regression?
Hi folks, I have a few headless qemu domains that use serial consoles. Typically i connect to the consoles with # virsh console domain However with 0.9.7 I get Connected to domain gentoo-base Escape character is ^] error: internal error character device (null) is not using a PTY I am still able to connect to the console if i go into the virt-manager gui and use the gui console, but I'd rather not do that as sometimes I don't have access to the gui. Is this a regression or am I expected to do some kind of migration on my domains. These are qemu guests running on Gentoo Linux. An xml dump of a typical domain follows: domain type='kvm' namebase/name uuidb4a528dd-4c31-03ca-bdde-795c4c38592d/uuid memory524288/memory currentMemory61440/currentMemory vcpu1/vcpu os type arch='x86_64' machine='pc-0.12'hvm/type kernel/var/lib/libvirt/images/vmlinuz/kernel cmdlineroot=/dev/vda1 rootfstype=ext4 quiet console=ttyS0 TERM=vt100/cmdline boot dev='hd'/ bootmenu enable='no'/ /os features acpi/ pae/ /features cpu match='exact' modelNehalem/model vendorIntel/vendor feature policy='require' name='tm2'/ feature policy='require' name='est'/ feature policy='require' name='monitor'/ feature policy='require' name='ss'/ feature policy='require' name='vme'/ feature policy='require' name='rdtscp'/ feature policy='require' name='ht'/ feature policy='require' name='ds'/ feature policy='require' name='pbe'/ feature policy='require' name='tm'/ feature policy='require' name='vmx'/ feature policy='require' name='ds_cpl'/ feature policy='require' name='xtpr'/ feature policy='require' name='acpi'/ feature policy='require' name='x2apic'/ /cpu clock offset='utc'/ on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/usr/bin/qemu-kvm/emulator disk type='file' device='disk' driver name='qemu' type='qcow2' cache='none'/ source file='/var/lib/libvirt/images/base.qcow'/ target dev='vda' bus='virtio'/ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /disk interface type='network' mac address='52:54:00:06:84:c0'/ source network='default'/ address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/ /interface serial type='pty' target port='0'/ /serial console type='pty' target type='serial' port='0'/ /console memballoon model='virtio' address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/ /memballoon /devices /domain Thanks in advance, -a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] nwfilter: simplify execution of ebiptables scripts
On 11/09/2011 11:39 AM, Stefan Berger wrote: On 11/09/2011 12:46 PM, Eric Blake wrote: It's not worth even worrying about a temporary file, unless we ever expect the script to exceed maximum command-line argument length limits. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI): Run the commands as an argument to /bin/sh, rather than worrying about a temporary file. (ebiptablesWriteToTempFile): Delete unused function. *status = 0; Here I had to insert: if (!virBufferUse(buf)) return 0; - cmds = virBufferContentAndReset(buf); - VIR_DEBUG(%s, NULLSTR(cmds)); - if (!cmds) - return 0; Ah, I see - the old code declared early success on no commands to run, while the new code passes an empty buffer to virCommand; and right now, virCommand has a bug that an empty buffer becomes NULL instead of an explicit empty argument (patch for that comming up next). But if there are no commands to run, then we can skip virCommand altogether (/bin/sh -c '' will always succeed). ACK with above nit fixed so it still works. I've pushed the two patches with that fixed; I decided to keep them separate for easier reversion of patch 2 if it turns out I was wrong and we ever hit an E2BIG error due to not having unlimited command line length. -- 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
[libvirt] [PATCH] command: handle empty buffer argument correctly
virBufferContentAndReset (intentionally) returns NULL for a buffer with no content, but it is feasible to invoke a command with an explicit empty string. * src/util/command.c (virCommandAddEnvBuffer): Reject empty string. (virCommandAddArgBuffer): Allow explicit empty argument. * tests/commandtest.c (test9): Test it. * tests/commanddata/test9.log: Adjust. --- As pointed out here: https://www.redhat.com/archives/libvir-list/2011-November/msg00435.html src/util/command.c | 13 - tests/commanddata/test9.log |4 +++- tests/commandtest.c | 13 - 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index c3ce361..f5effdf 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -983,6 +983,10 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf) virBufferFreeAndReset(buf); return; } +if (!virBufferUse(buf)) { +cmd-has_error = EINVAL; +return; +} cmd-env[cmd-nenv++] = virBufferContentAndReset(buf); } @@ -1092,7 +1096,14 @@ virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf) return; } -cmd-args[cmd-nargs++] = virBufferContentAndReset(buf); +cmd-args[cmd-nargs] = virBufferContentAndReset(buf); +if (!cmd-args[cmd-nargs]) +cmd-args[cmd-nargs] = strdup(); +if (!cmd-args[cmd-nargs]) { +cmd-has_error = ENOMEM; +return; +} +cmd-nargs++; } diff --git a/tests/commanddata/test9.log b/tests/commanddata/test9.log index 2607530..3a93c19 100644 --- a/tests/commanddata/test9.log +++ b/tests/commanddata/test9.log @@ -2,8 +2,10 @@ ARG:-version ARG:-log=bar.log ARG:arg1 ARG:arg2 -ARG:arg3 +ARG: ARG:arg4 +ARG:arg5 +ARG:arg6 ENV:DISPLAY=:0.0 ENV:HOME=/home/test ENV:HOSTNAME=test diff --git a/tests/commandtest.c b/tests/commandtest.c index dd6c248..efc48fe 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -352,11 +352,22 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir /commandhelper); const char* const args[] = { arg1, arg2, NULL }; +virBuffer buf = VIR_BUFFER_INITIALIZER; virCommandAddArg(cmd, -version); virCommandAddArgPair(cmd, -log, bar.log); virCommandAddArgSet(cmd, args); -virCommandAddArgList(cmd, arg3, arg4, NULL); +virCommandAddArgBuffer(cmd, buf); +virBufferAddLit(buf, arg4); +virCommandAddArgBuffer(cmd, buf); +virCommandAddArgList(cmd, arg5, arg6, NULL); + +if (virBufferUse(buf)) { +printf(Buffer not transferred\n); +virBufferFreeAndReset(buf); +virCommandFree(cmd); +return -1; +} if (virCommandRun(cmd, NULL) 0) { virErrorPtr err = virGetLastError(); -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] nwfilter: simplify execution of ebiptables scripts
On 11/09/2011 07:12 PM, Eric Blake wrote: ACK with above nit fixed so it still works. I've pushed the two patches with that fixed; I decided to keep them separate for easier reversion of patch 2 if it turns out I was wrong and we ever hit an E2BIG error due to not having unlimited command line length. Is 'getconf ARG_MAX' the command to use to determine the max. command line size? Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] nwfilter: simplify execution of ebiptables scripts
On 11/09/2011 06:43 PM, Stefan Berger wrote: On 11/09/2011 07:12 PM, Eric Blake wrote: ACK with above nit fixed so it still works. I've pushed the two patches with that fixed; I decided to keep them separate for easier reversion of patch 2 if it turns out I was wrong and we ever hit an E2BIG error due to not having unlimited command line length. Is 'getconf ARG_MAX' the command to use to determine the max. command line size? Thanks for the reminder. Yes, 'getconf ARG_MAX' is the current system limit, although getconf(1) isn't present on some systems (cygwin and mingw come rapidly to my mind). On my Fedora 14 system, I see a 2M limit for ARG_MAX; empirically, mingw has a much smaller limit of 32k. -- 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] [test-API][PATCH 2/3] clean keyword update in libvirt-test-api, mapper, proxy part
NACK - Original Message - From: Guannan Ren g...@redhat.com To: libvir-list@redhat.com Sent: Wednesday, November 9, 2011 12:38:38 PM Subject: [libvirt] [test-API][PATCH 2/3] clean keyword update in libvirt-test-api, mapper, proxy part --- libvirt-test-api.py |7 +++ mapper.py | 24 +++- proxy.py| 13 + 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/libvirt-test-api.py b/libvirt-test-api.py index 8a4bacd..171e13c 100644 --- a/libvirt-test-api.py +++ b/libvirt-test-api.py @@ -100,13 +100,20 @@ class LibvirtTestAPI(object): # extract the string of combination of # language, package, testname of a testcase. all_testcases_names = [] +prev_casename = '' for activity in activities_list: for testcase in activity: testcases_names = testcase.keys() if 'sleep' in testcases_names: testcases_names.remove('sleep') +if not cmp('clean', testcases_names[0]): +all_testcases_names += [prev_casename + :_clean] +continue + +prev_casename = testcases_names[0] all_testcases_names += testcases_names + unique_testcases_names = list(set(all_testcases_names)) # call and initilize proxy component to diff --git a/mapper.py b/mapper.py index 4f41230..affc510 100644 --- a/mapper.py +++ b/mapper.py @@ -24,20 +24,34 @@ class Mapper(object): def __init__(self, testcases_list): self.testcases_list = copy.deepcopy(testcases_list) -def get_package_tripped(self): +def package_casename_func_map(self): ^^^ The change of function name will cause problem when options cleanup enabled in test conf files, function name should also be udpated in env_clear.py After update function name in env_clear.py, another problem came out which the whole case will be rerun in cleanup process. So, update needed. Remove the package information to form a new dictionary tripped_cases_list = [] +prev_testcasename = '' +prev_testcases_params = '' for testcase in self.testcases_list: tripped_case = {} -casename = testcase.keys()[0] +testcase_name = testcase.keys()[0] +if : in testcase_name: +casename = testcase_name.split(:)[1] +func = casename -if casename == 'sleep': +if testcase_name == 'sleep': tripped_cases_list.append(testcase) continue -testcases_params = testcase.values()[0] -tripped_case[casename] = testcases_params +if testcase_name == 'clean': +func = casename + _clean +tripped_case[prev_testcasename + : + func] = prev_testcases_params +tripped_cases_list.append(tripped_case) +continue +testcases_params = testcase.values()[0] +tripped_case[testcase_name + : + func] = testcases_params tripped_cases_list.append(tripped_case) + +prev_testcasename = testcase_name +prev_testcases_params = testcases_params + return tripped_cases_list diff --git a/proxy.py b/proxy.py index a564230..f3664e3 100644 --- a/proxy.py +++ b/proxy.py @@ -30,17 +30,22 @@ class Proxy(object): def get_func_call_dict(self): for testcase_name in self.testcases_names: # Get programming package, casename -package = testcase_name.split(:)[0] -casename = testcase_name.split(:)[1] +elements = testcase_name.split(:) +package = elements[0] +casename = elements[1] +func = casename + +if len(elements) == 3: +keyword = elements[2] +func = casename + keyword # Dispatch functions funcs = getattr(self, get_call_dict) func_ref = None -func = casename func_ref = funcs(package, casename, func) # Construct function call dictionary -key = package + : + casename +key = package + : + casename + : + func self.func_dict[key] = func_ref return self.func_dict -- 1.7.1 -- 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] [RFC PATCH 0/8 v3] Summary on block IO throttle
On Thu, Nov 10, 2011 at 4:32 AM, Lei Li li...@linux.vnet.ibm.com wrote: Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotune domain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottle domain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotune domain device [--total_bytes_sec number] [--read_bytes_sec number] [--write_bytes_sec number] [--total_iops_sec number] [--read_iops_sec number] [--write_iops_sec number] [--config] [--live] [--current] Thanks Li Lei for the remaining works. Below is only one reminder. QEMU command line options have some limitations as below: (1) global bps limit. -drive bps=xxxin bytes/s (2) only read bps limit -drive bps_rd=xxx in bytes/s (3) only write bps limit -drive bps_wr=xxx in bytes/s (4) global iops limit -drive iops=xxx in ios/s (5) only read iops limit -drive iops_rd=xxxin ios/s (6) only write iops limit -drive iops_wr=xxxin ios/s (7) the combination of some limits. -drive bps=xxx,iops=xxx Known Limitations: (1) #1 can not be used with #2, #3 together (2) #4 can not be used with #5, #6 together daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py | 2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c | 101 src/conf/domain_conf.h | 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms | 2 src/qemu/qemu_command.c | 33 ++ src/qemu/qemu_driver.c
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
On 11/10/2011 04:43 AM, Eric Blake wrote: On 11/09/2011 01:32 PM, Lei Li wrote: This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c| 115 ++ src/libvirt_public.syms |2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ intvirDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { +unsigned long long total_bytes_sec; +unsigned long long read_bytes_sec; +unsigned long long write_bytes_sec; +unsigned long long total_iops_sec; +unsigned long long read_iops_sec; +unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure. Hi Eric, We consider using virTypeParameter as Adam suggested at first, but when I dig into the code, I found struct virTypedParameter is mainly used by memtune, blkiotune...etc, supported by cgroup mechanism, different from Block I/O throttling which is supported through qemu monitor command 'block_set_io_throttle'. Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command. -- Lei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/8 v3] Summary on block IO throttle
On 11/10/2011 12:45 PM, Zhi Yong Wu wrote: On Thu, Nov 10, 2011 at 4:32 AM, Lei Lili...@linux.vnet.ibm.com wrote: Changes since V2 - Implement the Python binding support for setting blkio throttling. - Implement --current --live --config options support to unify the libvirt API. - Add changes in docs and tests. - Some changes suggested by Adam Litke, Eric Blake, Daniel P. Berrange. - Change the XML schema. - API name to virDomain{Set, Get}BlockIoTune. - Parameters changed to make them more self-explanatory. - virsh command name to blkdeviotune. - And other fixups. Changes since V1 - Implement the support to get the block io throttling for a device as read only connection - QMP/HMP. - Split virDomainBlockIoThrottle into two separate functions virDomainSetBlockIoThrottle - Set block I/O limits for a device - Requires a connection in 'write' mode. - Limits (info) structure passed as an input parameter virDomainGetBlockIoThrottle - Get the current block I/O limits for a device - Works on a read-only connection. - Current limits are written to the output parameter (reply). - And Other fixups suggested by Adam Litke, Daniel P. Berrange. - For dynamically allocate the blkiothrottle struct, I will fix it when implement --current --live --config options support. Today libvirt supports the cgroups blkio-controller, which handles proportional shares and throughput/iops limits on host block devices. blkio-controller does not support network file systems (NFS) or other QEMU remote block drivers (curl, Ceph/rbd, sheepdog) since they are not host block devices. QEMU I/O throttling works with all types of drive and can be applied independently to each drive attached to a guest and supports throughput/iops limits. To help add QEMU I/O throttling support to libvirt, we plan to complete it with add new API virDomain{Set, Get}BlockIoThrottle(), new command 'blkdeviotune' and Python bindings. Notes: Now all the planed features were implemented (#1#2 were implemented by Zhi Yong Wu), the previous comments were all fixed up too. And the qemu part patches have been accepted upstream just now and are expected to be part of the QEMU 1.1 release, git tree from Zhi Yong: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block 1) Enable the blkio throttling in xml when guest is starting up. Add blkio throttling in xml as follows: disk type='file' device='disk' ... iotune total_bytes_secnnn/total_bytes_sec ... /iotune ... /disk 2) Enable blkio throttling setting at guest running time. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] \ [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] 3) The support to get the current block i/o throttling for a device - HMP/QMP. virsh blkiothrottledomain device total_bytes_sec: read_bytes_sec: write_bytes_sec: total_iops_sec: read_iops_sec: write_iops_sec: 4) Python binding support for setting blkio throttling. 5) --current --live --config options support to unify the libvirt API. virsh blkdeviotunedomain device [--total_bytes_secnumber] [--read_bytes_secnumber] [--write_bytes_secnumber] [--total_iops_secnumber] [--read_iops_secnumber] [--write_iops_secnumber] [--config] [--live] [--current] Thanks Li Lei for the remaining works. Below is only one reminder. QEMU command line options have some limitations as below: (1) global bps limit. -drive bps=xxxin bytes/s (2) only read bps limit -drive bps_rd=xxx in bytes/s (3) only write bps limit -drive bps_wr=xxx in bytes/s (4) global iops limit -drive iops=xxx in ios/s (5) only read iops limit -drive iops_rd=xxxin ios/s (6) only write iops limit -drive iops_wr=xxxin ios/s (7) the combination of some limits. -drive bps=xxx,iops=xxx Known Limitations: (1) #1 can not be used with #2, #3 together (2) #4 can not be used with #5, #6 together Yes, you might want to look into the code about domain_conf.c and qemu_driver.c part, I have already do the limitations on both XML and the command. daemon/remote.c | 87 +++ docs/formatdomain.html.in | 30 ++ docs/schemas/domaincommon.rng | 24 + include/libvirt/libvirt.h.in | 25 ++ python/generator.py |2 python/libvirt-override-api.xml | 16 + python/libvirt-override.c | 85 +++ src/conf/domain_conf.c| 101 src/conf/domain_conf.h| 12 src/driver.h | 18 + src/libvirt.c | 115 + src/libvirt_public.syms
Re: [libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
于 2011年11月10日 13:41, Lei Li 写道: On 11/10/2011 04:43 AM, Eric Blake wrote: On 11/09/2011 01:32 PM, Lei Li wrote: This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Zhi Yong Wuwu...@linux.vnet.ibm.com Signed-off-by: Lei Lili...@linux.vnet.ibm.com --- include/libvirt/libvirt.h.in | 26 + src/driver.h | 19 +++ src/libvirt.c | 115 ++ src/libvirt_public.syms | 2 + 4 files changed, 162 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa320b6..a79c35e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1640,6 +1640,32 @@ int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; This is not extensible. We've already learned the hard way that this MUST use virTypedParameter to make future additions painless, rather than hard-coding a particular structure. Hi Eric, We consider using virTypeParameter as Adam suggested at first, but when I dig into the code, I found struct virTypedParameter is mainly used by memtune, blkiotune...etc, supported by cgroup mechanism, different from Block I/O throttling which is supported through qemu monitor command 'block_set_io_throttle'. virTypedParameter is used between libvirt client and libvirtd. It isn't used between libvirtd and qemu. Qemu monitor send all the setting parameters at one time within qmp command made by qemuMonitorJSONMakeCommand, the union struct virTypeParameter is not very suitable for it. So we still use struct virDomainBlockIoTuneInfo to keep consistent for qemu monitor command. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix domjobabort regression
On Wed, Nov 09, 2011 at 09:29:23AM -0700, Eric Blake wrote: This reverts commit ef1065cf5ac; see also this bug report: https://bugzilla.redhat.com/show_bug.cgi?id=751900 In qemu 0.15.1 and earlier, during migration to file, the qemu_savevm_state_begin and qemu_savevm_state_iterate methods will both process as much migration data as possible until either 1. The file descriptor returns EAGAIN 2. The bandwidth rate limit is reached If we set the rate limit to ULONG_MAX, test 2 never becomes true. We're passing a plain file descriptor to QEMU and POSIX does not support EAGAIN on regular files / block devices, so test 1 never becomes true either. In the 'virsh save --bypass-cache' case, we pass a pipe instead of a regular fd, but using a pipe adds I/O overhead, so always passing a pipe just so qemu can see EAGAIN doesn't seem nice. The ultimate fix needs to come from qemu - background migration must ACK, it's a bit of a shame, but until we have a fix, it's more important to be able to abort a save to disk than to be able to save at a speed of more than 32MB/s (if the storage system support this) 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