Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On Feb 23, 2012, at 11:16 PM, Laine Stump wrote: On 02/22/2012 08:17 AM, D. Herrendoerfer wrote: From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name Add de-association handling for 802.1qbg (vepa) via lldpad netlink messages. Also adds the possibility to perform an association request without waiting for a confirmation. The main issue I see with this patch is with whitespace, but that can easily be fixed prior to pushing it, so ACK. Is the message format used by this patch, the absolute final version? (i.e. can we safely push it an know that it will be correct?) The patch to libvirt was picked this week. So yes, we can assume that the message will not be changed. But as always : Never say never ! The callback mechanism is not re-armed when libvirt is restarted now. The reason is: lldpad remembers who sent the associate by pid - since in theory there could be multiple agents performing associations. So if the libvirt pid changes, there is little we can do now. And thanks ! Dirk H Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name --- src/qemu/qemu_migration.c|2 +- src/util/virnetdevmacvlan.c | 315 +- src/util/virnetdevvportprofile.c | 33 +++-- src/util/virnetdevvportprofile.h |3 +- 4 files changed, 339 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12cfbde..31428f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { net-mac, virDomainNetGetActualDirectDev(net), def-uuid, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0) + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0) goto err_exit; } last_good_net = i; diff --git a/src/util/virnetdevmacvlan.c b/src/util/ virnetdevmacvlan.c index 25d0846..b3e3325 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, passthrough) #if WITH_MACVTAP - spurious whitespace change. # include stdint.h # include stdio.h # include errno.h @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include linux/if.h # include linux/if_tun.h +# include c-ctype.h + /* Older kernels lacked this enum value. */ # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU # define MACVLAN_MODE_PASSTHRU 8 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include virfile.h # include virnetlink.h # include virnetdev.h +# include virpidfile.h + # define MACVTAP_NAME_PREFIXmacvtap # define MACVTAP_NAME_PATTERN macvtap%d @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # define MACVLAN_NAME_PREFIXmacvlan # define MACVLAN_NAME_PATTERN macvlan%d + Another spurious whitespace change. /** * virNetDevMacVLanCreate: * @@ -445,6 +449,293 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, }; +/* Struct to hold the state and configuration of a 802.1qbg port */ +struct virNetlinkCallbackData { + char cr_ifname[64]; + virNetDevVPortProfilePtr virtPortProfile; + const unsigned char *macaddress; + const char *linkdev; + const unsigned char *vmuuid; + enum virNetDevVPortProfileOp vmOp; + unsigned int linkState; +}; + +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; + +#define INSTANCE_STRLEN 36 + +static int instance2str(const unsigned char *p, char *dst, size_t size) +{ +if (dst size INSTANCE_STRLEN) { +snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x- +%02x%02x-%02x%02x%02x%02x%02x%02x, +p[0], p[1], p[2], p[3], +p[4], p[5], p[6], p[7], +p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); +return 0; +} +return -1; +} + +# define LLDPAD_PID_FILE /var/run/lldpad.pid +# define VIRIP_PID_FILE /var/run/virip.pid + +/** + * virNetDevMacVLanVPortProfileCallback: + * + * @msg: The buffer containing the received netlink message + * @length: The length of the received netlink message. + * @peer: The netling sockaddr containing the peer information + * @handeled: Contains information if the message has been replied to yet + * @opaque: Contains vital information regarding the associated vm an interface + * + * This function is called when a netlink message is received. The function + * reads the message and responds if it is pertinent to the running VMs + * network interface. + */ + +static void
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
On Feb 24, 2012, at 3:29 PM, D. Herrendoerfer wrote: On Feb 23, 2012, at 11:16 PM, Laine Stump wrote: On 02/22/2012 08:17 AM, D. Herrendoerfer wrote: From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name Add de-association handling for 802.1qbg (vepa) via lldpad netlink messages. Also adds the possibility to perform an association request without waiting for a confirmation. The main issue I see with this patch is with whitespace, but that can easily be fixed prior to pushing it, so ACK. Is the message format used by this patch, the absolute final version? (i.e. can we safely push it an know that it will be correct?) The patch to libvirt was picked this week. So yes, we can assume that the message will not be changed. But as always : Never say never ! The patch to lldpad (Doh!) was picked ... The callback mechanism is not re-armed when libvirt is restarted now. The reason is: lldpad remembers who sent the associate by pid - since in theory there could be multiple agents performing associations. So if the libvirt pid changes, there is little we can do now. And thanks ! Dirk H Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name --- src/qemu/qemu_migration.c|2 +- src/util/virnetdevmacvlan.c | 315 +++ ++- src/util/virnetdevvportprofile.c | 33 +++-- src/util/virnetdevvportprofile.h |3 +- 4 files changed, 339 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12cfbde..31428f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { net-mac, virDomainNetGetActualDirectDev(net), def-uuid, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0) + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0) goto err_exit; } last_good_net = i; diff --git a/src/util/virnetdevmacvlan.c b/src/util/ virnetdevmacvlan.c index 25d0846..b3e3325 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, passthrough) #if WITH_MACVTAP - spurious whitespace change. # include stdint.h # include stdio.h # include errno.h @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include linux/if.h # include linux/if_tun.h +# include c-ctype.h + /* Older kernels lacked this enum value. */ # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU # define MACVLAN_MODE_PASSTHRU 8 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include virfile.h # include virnetlink.h # include virnetdev.h +# include virpidfile.h + # define MACVTAP_NAME_PREFIXmacvtap # define MACVTAP_NAME_PATTERN macvtap%d @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # define MACVLAN_NAME_PREFIXmacvlan # define MACVLAN_NAME_PATTERN macvlan%d + Another spurious whitespace change. /** * virNetDevMacVLanCreate: * @@ -445,6 +449,293 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, }; +/* Struct to hold the state and configuration of a 802.1qbg port */ +struct virNetlinkCallbackData { + char cr_ifname[64]; + virNetDevVPortProfilePtr virtPortProfile; + const unsigned char *macaddress; + const char *linkdev; + const unsigned char *vmuuid; + enum virNetDevVPortProfileOp vmOp; + unsigned int linkState; +}; + +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; + +#define INSTANCE_STRLEN 36 + +static int instance2str(const unsigned char *p, char *dst, size_t size) +{ +if (dst size INSTANCE_STRLEN) { +snprintf(dst, size, %02x%02x%02x%02x-%02x%02x-%02x%02x- +%02x%02x-%02x%02x%02x%02x%02x%02x, +p[0], p[1], p[2], p[3], +p[4], p[5], p[6], p[7], +p[8], p[9], p[10], p[11], p[12], p[13], p[14], p[15]); +return 0; +} +return -1; +} + +# define LLDPAD_PID_FILE /var/run/lldpad.pid +# define VIRIP_PID_FILE /var/run/virip.pid + +/** + * virNetDevMacVLanVPortProfileCallback: + * + * @msg: The buffer containing the received netlink message + * @length: The length of the received netlink message. + * @peer: The netling sockaddr containing the peer information + * @handeled: Contains information if the message has been replied to yet + * @opaque: Contains vital information regarding the associated vm an interface + * + * This function is called when a netlink message is received. The function + * reads the message and
[libvirt] [PATCH] Improve error reporting when virsh console is run without a TTY
From: Daniel P. Berrange berra...@redhat.com If attempting to run ssh root@somehost virsh console someguest You'll get an error 2012-02-15 13:11:47.683+: 4765: info : libvirt version: 0.9.10, package: 1.fc18 (Unknown, 2012-02-15-11:48:57, lettuce.camlab.fab.redhat.com) 2012-02-15 13:11:47.683+: 4765: error : vshRunConsole:320 : unable to get tty attributes: Invalid argument Connected to domain f16x86_64 Escape character is ^] There are several problems here - The actual error message is bad for users - We shouldn't rely on VIR_ERROR for this case - The prompt makes it look like we still connected because we didn't flush stdout. * virsh.c: Flush stdout before starting console and check for a valid tty --- tools/virsh.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3be86ed..b97a888 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -838,8 +838,14 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name) goto cleanup; } +if (!isatty(STDIN_FILENO)) { +vshError(ctl, %s, _(Cannot run interactive console without a controlling TTY)); +goto cleanup; +} + vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom)); vshPrintExtra(ctl, _(Escape character is %s\n), ctl-escapeChar); +fflush(stdout); if (vshRunConsole(dom, name, ctl-escapeChar) == 0) ret = true; -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Workaround python header file insanity
From: Daniel P. Berrange berra...@redhat.com The /usr/include/python/pyconfig.h file pollutes the global namespace with a huge number of HAVE_XXX and WITH_XXX defines. These change what we detected in our own config.h In particular if you try to build without DTrace, python's headers turn it back on with predictable fail. THe hack to workaround this is to rename WITH_DTRACE to WITH_DTRACE_PROBES to avoid the namespace clash --- configure.ac |4 ++-- daemon/Makefile.am |2 +- src/Makefile.am|4 ++-- src/internal.h |2 +- tests/Makefile.am |2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 732f4fe..262e63b 100644 --- a/configure.ac +++ b/configure.ac @@ -1378,10 +1378,10 @@ if test $with_dtrace != no ; then with_dtrace=yes fi if test $with_dtrace = yes; then -AC_DEFINE_UNQUOTED([WITH_DTRACE], 1, [whether DTrace static probes are available]) +AC_DEFINE_UNQUOTED([WITH_DTRACE_PROBES], 1, [whether DTrace static probes are available]) fi fi -AM_CONDITIONAL([WITH_DTRACE], [test $with_dtrace != no]) +AM_CONDITIONAL([WITH_DTRACE_PROBES], [test $with_dtrace != no]) dnl NUMA lib diff --git a/daemon/Makefile.am b/daemon/Makefile.am index e2c1357..db4abf5 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -109,7 +109,7 @@ libvirtd_LDADD =\ $(SASL_LIBS)\ $(POLKIT_LIBS) -if WITH_DTRACE +if WITH_DTRACE_PROBES libvirtd_LDADD += ../src/probes.o endif diff --git a/src/Makefile.am b/src/Makefile.am index d5f52a0..9b1921d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1284,7 +1284,7 @@ libvirt_la_CFLAGS = -DIN_LIBVIRT $(AM_CFLAGS) # picked out for us. libvirt_la_DEPENDENCIES = $(libvirt_la_BUILT_LIBADD) $(LIBVIRT_SYMBOL_FILE) -if WITH_DTRACE +if WITH_DTRACE_PROBES libvirt_la_BUILT_LIBADD += probes.o libvirt_la_DEPENDENCIES += probes.o nodist_libvirt_la_SOURCES = probes.h @@ -1521,7 +1521,7 @@ libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ $(LIBNL_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(RT_LIBS) \ ../gnulib/lib/libgnu.la -if WITH_DTRACE +if WITH_DTRACE_PROBES libvirt_lxc_LDADD += probes.o endif if WITH_SECDRIVER_SELINUX diff --git a/src/internal.h b/src/internal.h index fabcb52..3408541 100644 --- a/src/internal.h +++ b/src/internal.h @@ -247,7 +247,7 @@ # define VIR_DIV_UP(value, size) (((value) + (size) - 1) / (size)) -# if WITH_DTRACE +# if WITH_DTRACE_PROBES # ifndef LIBVIRT_PROBES_H # define LIBVIRT_PROBES_H # include probes.h diff --git a/tests/Makefile.am b/tests/Makefile.am index 3fb9e2f..9974c2f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,7 +29,7 @@ INCLUDES += \ endif PROBES_O = -if WITH_DTRACE +if WITH_DTRACE_PROBES PROBES_O += ../src/probes.o endif -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Workaround python header file insanity
On 02/24/2012 08:13 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com The /usr/include/python/pyconfig.h file pollutes the global namespace with a huge number of HAVE_XXX and WITH_XXX defines. These change what we detected in our own config.h In particular if you try to build without DTrace, python's headers turn it back on with predictable fail. THe hack to workaround this is to rename WITH_DTRACE to WITH_DTRACE_PROBES to avoid the namespace clash --- configure.ac |4 ++-- daemon/Makefile.am |2 +- src/Makefile.am|4 ++-- src/internal.h |2 +- tests/Makefile.am |2 +- 5 files changed, 7 insertions(+), 7 deletions(-) ACK. (And I really wish python would clean up their act). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Improve error reporting when virsh console is run without a TTY
On 02/24/2012 08:13 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com If attempting to run ssh root@somehost virsh console someguest You'll get an error 2012-02-15 13:11:47.683+: 4765: info : libvirt version: 0.9.10, package: 1.fc18 (Unknown, 2012-02-15-11:48:57, lettuce.camlab.fab.redhat.com) 2012-02-15 13:11:47.683+: 4765: error : vshRunConsole:320 : unable to get tty attributes: Invalid argument Connected to domain f16x86_64 Escape character is ^] There are several problems here - The actual error message is bad for users - We shouldn't rely on VIR_ERROR for this case - The prompt makes it look like we still connected because we didn't flush stdout. Did you also test 'ssh root@somehost virsh start --console someguest' to make sure that the guest is started, but the console sanely refused? +++ b/tools/virsh.c @@ -838,8 +838,14 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name) goto cleanup; } +if (!isatty(STDIN_FILENO)) { +vshError(ctl, %s, _(Cannot run interactive console without a controlling TTY)); +goto cleanup; +} + vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom)); vshPrintExtra(ctl, _(Escape character is %s\n), ctl-escapeChar); +fflush(stdout); At any rate, this looks sane. ACK. Hmm, we ought to ask gnulib if they will relicense the isatty() module (currently LGPLv3+); as isatty() on mingw has bugs worth working around. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0
On 02/24/2012 03:41 AM, Daniel P. Berrange wrote: On Fri, Feb 24, 2012 at 11:34:45AM +0100, Christophe Fergeau wrote: It's possible to disable SPICE TLS in qemu.conf. When this happens, libvirt ignores any SPICE TLS port or x509 directory that may have been set when it builds the qemu command line to use. However, it's not ignoring the secure channels that may have been set and adds tls-channel arguments to qemu command line. Current qemu versions don't report an error when this happens, and try to use TLS for the specified channels. This fixes bug #790436 Thanks for chasing this down. ACK, if we s/XML_ERROR/CONFIG_UNSUPPORTED/ in the those two error messages Changed and pushed. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] New QMP event interface (was Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event)
On 02/24/2012 10:56 AM, Luiz Capitulino wrote: On Fri, 24 Feb 2012 10:44:11 -0600 Anthony Liguorianth...@codemonkey.ws wrote: I'm asking because the conversion of events to the qapi is not too far away, but I think that using QOM will somewhat deprecate the code you have in the glib branch (besides having to wait for 1.2)? I have some vague ideas about what to do here. One thought would be to have a standard notifier mechanism in Object that was advertised as a property type. We could then provide an interface via QMP to [un]subscribe to a notifier property. This seems to be a good match with your previous ideas, implemented in the glib branch. But then subsystems/devices emitting events will have to be converted to QOM first... Well we need to keep the old events for compatibility, so it's really just about new events. I think that by end of 1.2, we would have all non-qdev subsystems converted to QOM also so this shouldn't be a problem in practice. I won't get to this until the 1.2 time frame though. My goals for 1.1 are to get qbus conversions merged and refactor IRQs/MemoryRegions to use QOM. If time permits, also refactor the PC to better use QOM. If someone wants to tackle events in QOM, I'd be happy to provide some suggestions on where to start. I'd like to hear about it, but I'm not sure when I'll start working on it. I've only thought about this roughly, but the problems that need to be solved are: 1) Have an object property that corresponds to a NotifierList (easy) 2) Figure out what it means to get and set a NotifierList. I think perhaps we could somehow turn this into subscribe/unsubscribe... We could take a plan9-like approach where get would return the canonical path of a new object that corresponded to a subscription to the event. This is nice because unsubscribing then just becomes a matter of destroying the subscription object. Once you had this subscription object, I think we would want a mechanism to connect the subscription with either a native function pointer or some mechanism that would let us translate that to QMP. Maybe we only connect with a native function pointer and use QAPI generation code to build a native function pointer that spits out a marshalled QObject. 3) We would need to think through the QMP interface for all of this. Given a path, we want to be able to subscribe to an event and unsubscribe from an event. We need to unsubscribe all subscribed events when the connection is lost. It would be nice to have convenience interfaces for doing things like subscribing to any event on a given type too including yet to be created objects. Cc'ing libvirt here to see if they have any additional requirements. Regards, Anthony Liguori -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] Fixed URI parsing
Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. Also there is one new syntax check function to prohibit these functions anywhere else. File changes: - src/util/viruri.h-- declaration - src/util/viruri.c-- definition - src/libvirt_private.syms -- symbol export - src/Makefile.am -- added source and header files - cfg.mk -- added sc_prohibit_xmlURI - all others -- ID name and include fixes --- v3: - wrappers moved to new files - syntax check added - virAsprintf used instead of nasty memory mechanics v2: - added virSaveURI for building back the original string cfg.mk |8 src/Makefile.am|3 +- src/datatypes.h|2 +- src/driver.h |2 +- src/esx/esx_driver.c |5 +- src/esx/esx_util.c |2 +- src/esx/esx_util.h |4 +- src/hyperv/hyperv_util.c |2 +- src/hyperv/hyperv_util.h |5 +- src/libvirt.c | 10 ++-- src/libvirt_private.syms |5 ++ src/libxl/libxl_driver.c |3 +- src/lxc/lxc_driver.c |3 +- src/openvz/openvz_driver.c |3 +- src/qemu/qemu_driver.c |2 +- src/qemu/qemu_migration.c |7 ++- src/remote/remote_driver.c |9 ++-- src/uml/uml_driver.c |3 +- src/util/qparams.c |3 +- src/util/viruri.c | 91 src/util/viruri.h | 18 + src/vbox/vbox_tmpl.c |3 +- src/vmx/vmx.c |6 +- src/xen/xen_driver.c |4 +- src/xen/xen_hypervisor.h |3 +- src/xen/xend_internal.c|4 +- src/xen/xend_internal.h|2 +- src/xenapi/xenapi_driver.c |2 +- src/xenapi/xenapi_utils.c |4 +- src/xenapi/xenapi_utils.h |4 +- 30 files changed, 174 insertions(+), 48 deletions(-) create mode 100644 src/util/viruri.c create mode 100644 src/util/viruri.h diff --git a/cfg.mk b/cfg.mk index dcf44bb..9759d87 100644 --- a/cfg.mk +++ b/cfg.mk @@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp: halt='use virXMLPropString, not xmlGetProp' \ $(_sc_search_regexp) +# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well +sc_prohibit_xmlURI: + @prohibit='\xml(ParseURI|SaveUri) *\(' \ + halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)' \ + $(_sc_search_regexp) + # ATTRIBUTE_UNUSED should only be applied in implementations, not # header declarations sc_avoid_attribute_unused_in_header: @@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \ exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$ +exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$ + exclude_file_name_regexp--sc_require_config_h = ^examples/ exclude_file_name_regexp--sc_require_config_h_first = ^examples/ diff --git a/src/Makefile.am b/src/Makefile.am index d5f52a0..e2542b1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -104,7 +104,8 @@ UTIL_SOURCES = \ util/virnetlink.c util/virnetlink.h \ util/virrandom.h util/virrandom.c \ util/virsocketaddr.h util/virsocketaddr.c \ - util/virtime.h util/virtime.c + util/virtime.h util/virtime.c \ + util/viruri.h util/viruri.c EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py diff --git a/src/datatypes.h b/src/datatypes.h index 47058ed..fc284d2 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -151,7 +151,7 @@ struct _virConnect { */ unsigned int magic; /* specific value to check */ unsigned int flags; /* a set of connection flags */ -xmlURIPtr uri; /* connection URI */ +virURIPtr uri; /* connection URI */ /* The underlying hypervisor driver and network driver. */ virDriverPtr driver; diff --git a/src/driver.h b/src/driver.h index d27fa99..b04b254 100644 --- a/src/driver.h +++ b/src/driver.h @@ -9,9 +9,9 @@ # include config.h # include unistd.h -# include libxml/uri.h # include internal.h +# include viruri.h /* * List of registered drivers numbers */ diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f5e1cc7..b6b22f8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -44,6 +44,7 @@ #include esx_vi.h #include esx_vi_methods.h #include esx_util.h +#include
Re: [libvirt] [PATCH v3] Fixed URI parsing
On 02/24/2012 11:09 AM, Martin Kletzander wrote: Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. Also there is one new syntax check function to prohibit these functions anywhere else. +++ b/src/libvirt_private.syms @@ -1430,6 +1430,11 @@ virTypedParameterArrayValidate; virTypedParameterAssign; +# viruri.h +virURIParse; +virURIFormat; Swap these two lines. +xmlURIPtr +virURIParse(const char *uri) +{ +xmlURIPtr ret = xmlParseURI(uri); + +/* First check: does it even make sense to jump inside */ +if (ret != NULL +ret-server != NULL +ret-server[0] == '[') { +size_t length = strlen(ret-server); + +/* We want to modify the server string only if there are + * square brackets on both ends and inside there is IPv6 + * address. Otherwise we could make a mistake by modifying + * something else than IPv6 address. */ s/else than/other than an/ +unsigned char * +virURIFormat(xmlURIPtr uri) +{ +char *tmpserver = NULL, *backupserver = uri-server; NULL deref... +unsigned char *ret; + +/* First check: does it make sense to do anything */ +if (uri != NULL ...since you allow uri == NULL on input. Reorder the assignment to backupserver to come after you know uri is not NULL. +uri-server != NULL +strchr(uri-server, ':') != NULL) { + +if (virAsprintf(tmpserver, [%s], uri-server) == -1) It's more idiomatic to use ' 0', not '== -1'. +++ b/src/util/viruri.h @@ -0,0 +1,18 @@ +/* + * viruri.h: internal definitions used for URI parsing. Needs a copyright header. ACK with those nits fixed; I think we're close enough that you can push without having to get a review on a v4. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Virt io blk buffer
On 02/23/2012 11:53 PM, Pankaj Rawat wrote: Hi all Can any one tell what is the default Vitio blk IO Buffer size You'll probably get better response on the qemu lists, since virtio is part of qemu. And can we change it ? if yes how? Libvirt does not currently expose anything to change this, other than the fact that _if_ qemu supports a way to change it on the command line, you can use qemu:command in your XML to add the extra arguments; or if qemu supports it via a monitor command, you can use 'virsh qemu-monitor-command' to change it at runtime. But again, I don't know enough about the situation to know if qemu even exposes a parameterizable block size for virtio, since no one has previously asked for libvirt to start encoding it into its XML as a formal feature. The contents of this e-mail and any attachment(s) are confidential and Disclaimers like this are unenforceable when posting to publicly archived lists, and it is considered poor netiquette to leave them in. I'd recommend sending from a private personal account if your employer insists on adding this garbage to your mail from work. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fixed URI parsing
Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. Also there is one new syntax check function to prohibit these functions anywhere else. File changes: - src/util/viruri.h-- declaration - src/util/viruri.c-- definition - src/libvirt_private.syms -- symbol export - src/Makefile.am -- added source and header files - cfg.mk -- added sc_prohibit_xmlURI - all others -- ID name and include fixes --- v4: - fixed NULL pointer dereference - fixed minor typo - copyright added to header v3: - wrappers moved to new files - syntax check added - virAsprintf used instead of nasty memory mechanics v2: - added virSaveURI for building back the original string cfg.mk |8 src/Makefile.am|3 +- src/datatypes.h|2 +- src/driver.h |2 +- src/esx/esx_driver.c |5 +- src/esx/esx_util.c |2 +- src/esx/esx_util.h |4 +- src/hyperv/hyperv_util.c |2 +- src/hyperv/hyperv_util.h |5 +- src/libvirt.c | 10 ++-- src/libvirt_private.syms |5 ++ src/libxl/libxl_driver.c |3 +- src/lxc/lxc_driver.c |3 +- src/openvz/openvz_driver.c |3 +- src/qemu/qemu_driver.c |2 +- src/qemu/qemu_migration.c |7 ++- src/remote/remote_driver.c |9 ++-- src/uml/uml_driver.c |3 +- src/util/qparams.c |3 +- src/util/viruri.c | 93 src/util/viruri.h | 22 ++ src/vbox/vbox_tmpl.c |3 +- src/vmx/vmx.c |6 +- src/xen/xen_driver.c |4 +- src/xen/xen_hypervisor.h |3 +- src/xen/xend_internal.c|4 +- src/xen/xend_internal.h|2 +- src/xenapi/xenapi_driver.c |2 +- src/xenapi/xenapi_utils.c |4 +- src/xenapi/xenapi_utils.h |4 +- 30 files changed, 180 insertions(+), 48 deletions(-) create mode 100644 src/util/viruri.c create mode 100644 src/util/viruri.h diff --git a/cfg.mk b/cfg.mk index dcf44bb..9759d87 100644 --- a/cfg.mk +++ b/cfg.mk @@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp: halt='use virXMLPropString, not xmlGetProp' \ $(_sc_search_regexp) +# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well +sc_prohibit_xmlURI: + @prohibit='\xml(ParseURI|SaveUri) *\(' \ + halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)' \ + $(_sc_search_regexp) + # ATTRIBUTE_UNUSED should only be applied in implementations, not # header declarations sc_avoid_attribute_unused_in_header: @@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \ exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$ +exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$ + exclude_file_name_regexp--sc_require_config_h = ^examples/ exclude_file_name_regexp--sc_require_config_h_first = ^examples/ diff --git a/src/Makefile.am b/src/Makefile.am index d5f52a0..e2542b1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -104,7 +104,8 @@ UTIL_SOURCES = \ util/virnetlink.c util/virnetlink.h \ util/virrandom.h util/virrandom.c \ util/virsocketaddr.h util/virsocketaddr.c \ - util/virtime.h util/virtime.c + util/virtime.h util/virtime.c \ + util/viruri.h util/viruri.c EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \ $(srcdir)/util/virkeycode-mapgen.py diff --git a/src/datatypes.h b/src/datatypes.h index 47058ed..fc284d2 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -151,7 +151,7 @@ struct _virConnect { */ unsigned int magic; /* specific value to check */ unsigned int flags; /* a set of connection flags */ -xmlURIPtr uri; /* connection URI */ +virURIPtr uri; /* connection URI */ /* The underlying hypervisor driver and network driver. */ virDriverPtr driver; diff --git a/src/driver.h b/src/driver.h index d27fa99..b04b254 100644 --- a/src/driver.h +++ b/src/driver.h @@ -9,9 +9,9 @@ # include config.h # include unistd.h -# include libxml/uri.h # include internal.h +# include viruri.h /* * List of registered drivers numbers */ diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f5e1cc7..b6b22f8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -44,6
Re: [libvirt] Correct a check for capacity arg of storageVolumeResize()
On 02/23/2012 08:00 PM, Zeeshan Ali (Khattak) wrote: From: Zeeshan Ali (Khattak) zeesha...@gnome.org --- src/storage/storage_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index df0e291..641944d 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1758,7 +1758,7 @@ storageVolumeResize(virStorageVolPtr obj, goto out; } -if (abs_capacity vol-allocation + pool-def-available) { +if (abs_capacity vol-capacity + pool-def-available) { I'm not sure this is right. If you allow for sparse files and over-commitment of capacity, then sparsely resizing to something that has too much capacity but still fits within allocation limits would be a reasonable that should not be rejected. I think it is more likely that we have several bugs in this area, in being too lax about which values we are actually checking. In fact, is the only reason we are checking for overflow is to avoid wasting time doing a partial allocation just to learn at the end that we would hit ENOSPACE? Adding to your commit message with the actual scenario you used to trigger the problem, and why your fix is correct, will go a long way in convincing me that this is needed. virStorageReportError(VIR_ERR_INVALID_ARG, Meanwhile, I just noticed this error code is wrong - the argument was syntactically valid, so the real problem is that we are out of space, which fits better as VIR_ERR_OPERATION_FAILED, or possibly a new error message specific to out-of-space errors. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/2] Add de-association handling to macvlan code
While going through this code to clean up the white-space problems, I found 3 issues that need to be addressed before I can push it. Sorry I missed these before. If you can base the new (and I hope final! :-) version on the version where I've already corrected the whitespace, that would be very helpful (otherwise I'll need to merge back with my updated version, a mechanical, but time consuming process). Rather than post them to the mailing list, I've pushed my updated versions of your patches to the lldpad2 branch of my staging repo on gitorious: git://gitorious.org/~laine/libvirt/laine-staging.git You should be able to get that branch into your local repo like this: git remote add laine-staging \ git://gitorious.org/~laine/libvirt/laine-staging.git git fetch laine-staging git checkout laine-staging/llpad2 You can then cherry-pick those commits into your own branch, or create a local branch based on it, then just squash in changes addressing the three problems I point out below. Once these are fixed, I will ACK and push it. Thanks! On 02/22/2012 08:17 AM, D. Herrendoerfer wrote: From: D. Herrendoerfer d.herrendoer...@herrendoerfer.name Add de-association handling for 802.1qbg (vepa) via lldpad netlink messages. Also adds the possibility to perform an association request without waiting for a confirmation. Signed-off-by: D. Herrendoerfer d.herrendoer...@herrendoerfer.name --- src/qemu/qemu_migration.c|2 +- src/util/virnetdevmacvlan.c | 315 +- src/util/virnetdevvportprofile.c | 33 +++-- src/util/virnetdevvportprofile.h |3 +- 4 files changed, 339 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12cfbde..31428f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2567,7 +2567,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) { net-mac, virDomainNetGetActualDirectDev(net), def-uuid, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) 0) + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH, false) 0) goto err_exit; } last_good_net = i; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 25d0846..b3e3325 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -46,7 +46,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, passthrough) #if WITH_MACVTAP - # include stdint.h # include stdio.h # include errno.h @@ -57,6 +56,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include linux/if.h # include linux/if_tun.h +# include c-ctype.h + /* Older kernels lacked this enum value. */ # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU # define MACVLAN_MODE_PASSTHRU 8 @@ -68,6 +69,8 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # include virfile.h # include virnetlink.h # include virnetdev.h +# include virpidfile.h + # define MACVTAP_NAME_PREFIX macvtap # define MACVTAP_NAME_PATTERNmacvtap%d @@ -75,6 +78,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST, # define MACVLAN_NAME_PREFIX macvlan # define MACVLAN_NAME_PATTERNmacvlan%d + /** * virNetDevMacVLanCreate: * @@ -445,6 +449,293 @@ static const uint32_t modeMap[VIR_NETDEV_MACVLAN_MODE_LAST] = { [VIR_NETDEV_MACVLAN_MODE_PASSTHRU] = MACVLAN_MODE_PASSTHRU, }; +/* Struct to hold the state and configuration of a 802.1qbg port */ +struct virNetlinkCallbackData { + char cr_ifname[64]; Issue 1, part 1: This should either be IFNAMSIZ bytes long, or be a char* (pointing to separately allocated data) instead of a fixed size. This string is copied into strings that are sent to the kernel for interface names, and interface names have a maximum size of IFNAMSIZE (16); if you need consistency of the name in all places (even in log messages), then you should limit it at the source, but otherwise there's no sense in checking string sizes both at this level and then again at the lower level. My preference would be a char*, I think - leave it to the lower levels to decide if it needs to be chopped. + virNetDevVPortProfilePtr virtPortProfile; + const unsigned char *macaddress; + const char *linkdev; + const unsigned char *vmuuid; + enum virNetDevVPortProfileOp vmOp; + unsigned int linkState; +}; + +typedef struct virNetlinkCallbackData *virNetlinkCallbackDataPtr; + +#define INSTANCE_STRLEN 36 + +static int instance2str(const unsigned char *p, char *dst, size_t size) +{ +if (dst size INSTANCE_STRLEN) { +
[libvirt] [PATCH] virsh: fix informational message in iface-bridge command
See: https://bugzilla.redhat.com/show_bug.cgi?id=797066 The position of the bridge name and ethernet device name were accidentally swapped in the message informing of success creating the bridge. --- tools/virsh.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index b97a888..66bbb0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8974,7 +8974,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) } vshPrint(ctl, _(Created bridge %s with attached device %s\n), - if_name, br_name); + br_name, if_name); /* start it up unless requested not to */ if (!nostart) { -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix informational message in iface-bridge command
On 02/24/2012 02:34 PM, Laine Stump wrote: See: https://bugzilla.redhat.com/show_bug.cgi?id=797066 The position of the bridge name and ethernet device name were accidentally swapped in the message informing of success creating the bridge. --- tools/virsh.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index b97a888..66bbb0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8974,7 +8974,7 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) } vshPrint(ctl, _(Created bridge %s with attached device %s\n), - if_name, br_name); + br_name, if_name); /* start it up unless requested not to */ if (!nostart) { I pushed this as trivial :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] Fixed URI parsing
On Fri, Feb 24, 2012 at 07:09:49PM +0100, Martin Kletzander wrote: +xmlURIPtr +virURIParse(const char *uri) +unsigned char * +virURIFormat(xmlURIPtr uri) The data types here are wrong compared to the header. Also the return value should not be unsigned - that is libxml2 bad practice we shouldn't copy diff --git a/src/util/viruri.h b/src/util/viruri.h new file mode 100644 index 000..1315488 --- /dev/null +++ b/src/util/viruri.h @@ -0,0 +1,18 @@ +/* + * viruri.h: internal definitions used for URI parsing. + */ + +#ifndef __VIR_URI_H__ +# define __VIR_URI_H__ + +# include libxml/uri.h + +# include internal.h + +typedef xmlURIvirURI; +typedef xmlURIPtr virURIPtr; + +virURIPtrvirURIParse(const char *uri); +unsigned char * virURIFormat(virURIPtr uri); + +#endif /* __VIR_URI_H__ */ 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] Correct a check for capacity arg of storageVolumeResize()
On Fri, Feb 24, 2012 at 8:58 PM, Eric Blake ebl...@redhat.com wrote: On 02/23/2012 08:00 PM, Zeeshan Ali (Khattak) wrote: From: Zeeshan Ali (Khattak) zeesha...@gnome.org --- src/storage/storage_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index df0e291..641944d 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1758,7 +1758,7 @@ storageVolumeResize(virStorageVolPtr obj, goto out; } - if (abs_capacity vol-allocation + pool-def-available) { + if (abs_capacity vol-capacity + pool-def-available) { I'm not sure this is right. If you allow for sparse files and over-commitment of capacity, then sparsely resizing to something that has too much capacity but still fits within allocation limits would be a reasonable that should not be rejected. I think it is more likely that we have several bugs in this area, in being too lax about which values we are actually checking. In fact, is the only reason we are checking for overflow is to avoid wasting time doing a partial allocation just to learn at the end that we would hit ENOSPACE? Adding to your commit message with the actual scenario you used to trigger the problem, and why your fix is correct, will go a long way in convincing me that this is needed. I got a volume with '1G' allocation and '10G' capacity. The available space in the parent pool is '5G'. With the current check for overcapacity, I can only try to resize to = '6G'. You see the problem? virStorageReportError(VIR_ERR_INVALID_ARG, Meanwhile, I just noticed this error code is wrong - the argument was syntactically valid, so the real problem is that we are out of space, which fits better as VIR_ERR_OPERATION_FAILED, or possibly a new error message specific to out-of-space errors. K, i'll submit a fix for that. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed service handling in specfile
On 02/24/2012 04:28 AM, Martin Kletzander wrote: After adding the libvirt-guests service into usual runlevels, we used to start the libvirt-guests service. However this is usually not a good practice. As mentioned on fedoraproject wiki, the installations can be in changeroots, in an installer context, or in other situations where we don't want the services autostarted. --- libvirt.spec.in |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 67cde23..072fd8e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1027,14 +1027,6 @@ fi %if %{with_systemd} %else /sbin/chkconfig --add libvirt-guests -if [ $1 -ge 1 ]; then -level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2) -if /sbin/chkconfig --levels $level libvirt-guests; then -# this doesn't do anything but allowing for libvirt-guests to be -# stopped on the first shutdown -/sbin/service libvirt-guests start /dev/null 21 || true -fi -fi %endif ACK and pushed. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fixed URI parsing
On 02/24/2012 11:48 AM, Martin Kletzander wrote: Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. Also there is one new syntax check function to prohibit these functions anywhere else. File changes: - src/util/viruri.h-- declaration - src/util/viruri.c-- definition - src/libvirt_private.syms -- symbol export - src/Makefile.am -- added source and header files - cfg.mk -- added sc_prohibit_xmlURI - all others -- ID name and include fixes +++ b/cfg.mk @@ -457,6 +457,12 @@ sc_prohibit_xmlGetProp: halt='use virXMLPropString, not xmlGetProp' \ $(_sc_search_regexp) +# xml(ParseURI|SaveUri) doesn't handle IPv6 URIs well +sc_prohibit_xmlURI: + @prohibit='\xml(ParseURI|SaveUri) *\(' \ + halt='use virURI(Parse|Format), not xml(ParseURI|SaveUri)' \ + $(_sc_search_regexp) + # ATTRIBUTE_UNUSED should only be applied in implementations, not # header declarations sc_avoid_attribute_unused_in_header: @@ -758,6 +764,8 @@ exclude_file_name_regexp--sc_prohibit_strncpy = \ exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/xml\.c$$ +exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$ + ACK; and pushed with these further changes squashed in to address Dan's comments on v3. diff --git i/src/util/viruri.c w/src/util/viruri.c index 0cbfc5a..6c4dfe3 100644 --- i/src/util/viruri.c +++ w/src/util/viruri.c @@ -25,10 +25,10 @@ * * @returns the parsed uri object with some fixes */ -xmlURIPtr +virURIPtr virURIParse(const char *uri) { -xmlURIPtr ret = xmlParseURI(uri); +virURIPtr ret = xmlParseURI(uri); /* First check: does it even make sense to jump inside */ if (ret != NULL @@ -62,12 +62,12 @@ virURIParse(const char *uri) * * @returns the constructed uri as a string */ -unsigned char * +char * virURIFormat(xmlURIPtr uri) { char *backupserver = NULL; char *tmpserver = NULL; -unsigned char *ret; +char *ret; /* First check: does it make sense to do anything */ if (uri != NULL @@ -81,7 +81,7 @@ virURIFormat(xmlURIPtr uri) uri-server = tmpserver; } -ret = xmlSaveUri(uri); +ret = (char *) xmlSaveUri(uri); /* Put the fixed version back */ if (tmpserver) { diff --git i/src/util/viruri.h w/src/util/viruri.h index b2b0ca8..5215e42 100644 --- i/src/util/viruri.h +++ w/src/util/viruri.h @@ -16,7 +16,7 @@ typedef xmlURIvirURI; typedef xmlURIPtr virURIPtr; -virURIPtrvirURIParse(const char *uri); -unsigned char * virURIFormat(virURIPtr uri); +virURIPtr virURIParse(const char *uri); +char *virURIFormat(virURIPtr uri); #endif /* __VIR_URI_H__ */ diff --git i/src/libvirt.c w/src/libvirt.c index a4ee63d..cbb4119 100644 --- i/src/libvirt.c +++ w/src/libvirt.c @@ -1729,7 +1729,7 @@ virConnectGetURI (virConnectPtr conn) return NULL; } -name = (char *)virURIFormat(conn-uri); +name = virURIFormat(conn-uri); if (!name) { virReportOOMError(); goto error; diff --git i/src/remote/remote_driver.c w/src/remote/remote_driver.c index 8fb46e1..bcd78ee 100644 --- i/src/remote/remote_driver.c +++ w/src/remote/remote_driver.c @@ -504,7 +504,7 @@ doRemoteOpen (virConnectPtr conn, transport_str[-1] = '\0'; } -name = (char *) virURIFormat (tmpuri); +name = virURIFormat(tmpuri); #ifdef HAVE_XMLURI_QUERY_RAW VIR_FREE(tmpuri.query_raw); -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 0/7] Console corruption patchset
On 02/23/2012 07:03 AM, Peter Krempa wrote: Yet another spin of the console corruption patches. Current state: * 1/7 - pidfile: Make checking binary path in virPidFileRead optional - No changes to v4. - ACKed by Eric * 2/7 - Add flags for virDomainOpenConsole - No changes to v4. - ACKed by Eric * 3/7 - virsh: add support for VIR_DOMAIN_CONSOLE_* flags - No changes to v4. - ACKed by Eric * 4/7 - fdstream: Emit stream abort callback even if poll() doesnt. - Tweaked according to Eric's review * 5/7 - fdstream: Add internal callback on stream close - Tweaked according to Eric's review * 6/7 - util: Add helpers for safe domain console operations - Tweaked according to Eric's review * 7/7 - qemu: Add ability to abort existing console while creating new one - No changes to v4. - ACKed by Eric ACK series if you make some changes to 6/7. At this point, I don't know if we're going to get a timely review from Dan, so I'm comfortable pushing into the tree now to lengthen the testing time we can give this prior to the 0.9.11 freeze. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 6/7] util: Add helpers for safe domain console operations
On 02/23/2012 07:03 AM, Peter Krempa wrote: This patch adds a set of functions used in creating console streams for domains using PTYs and ensures mutually exclusive access to the PTYs. If mutualy exclusive access is not used, two clients may open the same s/mutualy/mutually/ console, which results in corruption on both clients as both of them race to read data from the PTY. Diff to v4: - fixed typos (traditionally) - fixed configure.ac to work with automaticaly used values - tweaked configure output line to line up with others without moving them - using STRSKIP to skip the start of the string instead of possibly skipping in the middle of a string - fixed whitespace problems - changed data type for pids to pid_t and tweaked printing formatters - On failure to initialise mutex in virConsoleAlloc don't skip to virConsoleFree - added comment to clarify why it's needed to unregister the callback handler - Added OOM error reporting on some error paths. Looks better. Note to v4 review: I changed the default value for the path of the lock files to auto so it will get built with /var/lock on linux machines, so no change to the spec file is needed. auto should default to 'no' rather than error out on systems where we don't know. +dnl UUCP style file locks for PTY consoles +if test $with_console_lock_files != no; then + if test $with_console_lock_files = yes; then +AC_MSG_ERROR([console lock files requested, but no path given]) + elif test $with_console_lock_files = auto; then +dnl Default locations for platforms +if test $with_linux = yes; then + with_console_lock_files=/var/lock +fi + fi + if test $with_console_lock_files = auto; then +AC_MSG_ERROR([You must specify path for the lock files on this platform]) + fi I'd write this hunk as: if test $with_console_lock_files != no; then case $with_console_lock_files in yes | auto) dnl Default locations for platforms, or disable if unknown if test $with_linux = yes; then with_console_lock_files=/var/lock elif test $with_console_lock_files = auto with_console_lock_files=no fi;; esac if test $with_console_lock_files = yes; then AC_MSG_ERROR([You must specify path for the lock files on this platform]) fi +int virConsoleOpen(virConsolesPtr cons, + const char *pty, + virStreamPtr st, + bool force) +{ +virConsoleStreamInfoPtr cbdata = NULL; +virStreamPtr savedStream; +int ret; + +virMutexLock(cons-lock); + +if ((savedStream = virHashLookup(cons-hash, pty))) { +if (!force) { + /* entry found, console is busy */ +virMutexUnlock(cons-lock); +return 1; + } else { + /* terminate existing connection */ + /* The internal close callback handler needs to lock cons-lock to +* remove the aborted stream from the hash. This would cause a +* deadlock as we would try to enter the lock twice from the very +* same thread. We need to unregister the callback and abort the +* stream manualy before we create a new console connection. s/manualy/manually/ ACK with those changes. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] qemu: Implement virDomainPMWakeup API
On 02/23/2012 01:44 AM, Michal Privoznik wrote: On 15.02.2012 16:04, Michal Privoznik wrote: using 'system-wakeup' monitor command. It is supported only in JSON, as we are enabling it if possible. Moreover, this command is available in qemu-1.1+ which definitely has JSON. --- src/qemu/qemu_driver.c | 55 ++ src/qemu/qemu_monitor.c | 19 ++ src/qemu/qemu_monitor.h |2 + src/qemu/qemu_monitor_json.c | 21 src/qemu/qemu_monitor_json.h |2 + 5 files changed, 99 insertions(+), 0 deletions(-) Ping? Eric, it seems to me like you've forgotten this last patch. Indeed, it fell off my stack of most-recently-pinged patches. Reviewing now, and thanks for the ping... using 'system-wakeup' monitor command. It is supported only in JSON, as we are enabling it if possible. Moreover, this command is available in qemu-1.1+ which definitely has JSON. --- src/qemu/qemu_driver.c | 55 ++ src/qemu/qemu_monitor.c | 19 ++ src/qemu/qemu_monitor.h |2 + src/qemu/qemu_monitor_json.c | 21 src/qemu/qemu_monitor_json.h |2 + 5 files changed, 99 insertions(+), 0 deletions(-) +static int qemuDomainPMWakeup(virDomainPtr dom, + unsigned int flags) Style nit - we aren't very consistent on whether function names begin on line 1, but qemu_driver tends to use: static int qemuDomainPMWakeup(virDomainPtr dom, unsigned int flags) +++ b/src/qemu/qemu_monitor_json.c @@ -3492,3 +3492,24 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, virJSONValueFree(result); return ret; } + +int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon) +{ +int ret = -1; +virJSONValuePtr cmd = NULL; +virJSONValuePtr reply = NULL; + +cmd = qemuMonitorJSONMakeCommand(system_wakeup, NULL); Seems so simple :) ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: update to latest gnulib
It's been a while, and we're between releases, so now's as good a time as any to resync. I didn't notice any showstopper bugs being fixed, but we definitely get some improvements, such as tighter syntax checks. * .gnulib: Update to latest. * bootstrap: Resync. * cfg.mk (sc_prohibit_strncmp): Copy upstream changes to sc_prohibit_strcmp. --- * .gnulib e9e8aba...be29134 (47): stdalign: @samp - @code in doc for consistency stdnoreturn: new module regex: fix false multibyte matches in some regular expressions maint.mk: tell sc_prohibit_strcmp to ding 0 == strcmp (...), too streq: Rename macro. regex: fix typo in definition of MIN acl: Don't use ACL_CNT and similar ops, since they are unreliable. acl: Don't use GETACLCNT and similar ops, since they are unreliable. acl: Fix endless loop on Solaris with vxfs. acl: Fix copy-acl test failure on Solaris 11 2011-11. acl: Update doc references. Fix test failure in many locales on Solaris 11. gnulib-tool: Improve usage message. autoupdate README-release: make it easier to execute commands GNUmakefile: simplify detection of unconfigured trees autoupdate autoupdate autoupdate gnulib-tool: Doc fix. bootstrap: don't exit 0 upon gnulib-tool failure README-release: various improvements autoupdate maint: replace FSF snail-mail addresses with URLs README-release: capitalize a word and split a line fatal-signal: use C prototypes (with explicit void). regex: spelling fix regex: rely on stdint.h for SIZE_MAX regex: merge glibc changes maint.mk: also prohibit lower-case @var@ autoupdate maint: spelling fixes canonicalize: avoid uninitialized memory use isatty: Fix test failure of ptsname_r on native Windows. spawn-pipe tests: Fix a NULL program name in a diagnostic. nonblocking-socket tests: Fix a NULL program name in a diagnostic. nonblocking-pipe tests: Fix a NULL program name in a diagnostic. canonicalize-lgpl: fix // handling canonicalize: fix // handling ioctl: Fix test failure on native Windows. fsync: Avoid test failure on native Windows. * lib/sys_select.in.h [OpenBSD]: When /usr/include/pthread.h is sys_select: Avoid syntax error on OpenBSD 5.0. get-rusage-as, get-rusage-data tests: Avoid test failure with gcc-4.7. stdioext: Fix last commit. stdioext: Add tentative support for Plan9. file-has-acl: suppress a warning from gcc -Wsuggest-attribute=const .gnulib |2 +- bootstrap |6 +++--- cfg.mk|3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.gnulib b/.gnulib index e9e8aba..be29134 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit e9e8aba12af3c903edd422fa036a356c5b2f313a +Subproject commit be29134ddd011e6bcf1d73b4ae3d7ee7da60276f diff --git a/bootstrap b/bootstrap index 6910abf..31eb651 100755 --- a/bootstrap +++ b/bootstrap @@ -1,6 +1,6 @@ #! /bin/sh # Print a version string. -scriptversion=2012-01-21.16; # UTC +scriptversion=2012-02-11.09; # UTC # Bootstrap this package from checked-out sources. @@ -423,7 +423,7 @@ check_versions() { $use_git || continue fi # Honor $APP variables ($TAR, $AUTOCONF, etc.) -appvar=`echo $app | tr '[a-z]-' '[A-Z]_'` +appvar=`echo $app | LC_ALL=C tr '[a-z]-' '[A-Z]_'` test $appvar = TAR appvar=AMTAR case $appvar in GZIP) ;; # Do not use $GZIP: it contains gzip options. @@ -604,7 +604,7 @@ if $bootstrap_sync; then fi gnulib_tool=$GNULIB_SRCDIR/gnulib-tool -$gnulib_tool || exit +$gnulib_tool || exit $? # Get translations. diff --git a/cfg.mk b/cfg.mk index 9759d87..ac6c527 100644 --- a/cfg.mk +++ b/cfg.mk @@ -346,8 +346,9 @@ sc_prohibit_access_xok: # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0. +snp_ = strncmp *\(.+\) sc_prohibit_strncmp: - @grep -nE '! *str''ncmp *\(|\str''ncmp *\(.+\) *[!=]=' \ + @grep -nE '! *strncmp *\(|\$(snp_) *[!=]=|[!=]= *$(snp_)' \ $$($(VC_LIST_EXCEPT)) \ | grep -vE ':# *define STR(N?EQLEN|PREFIX)\(' \ { echo '$(ME): use STREQLEN or STRPREFIX instead of str''ncmp' \ -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: unescape HMP commands before converting them to json
QMP commands don't need to be escaped since converting them to json also escapes special characters. When a QMP command fails, however, libvirt falls back to HMP commands. These fallback functions (qemuMonitorText*) do their own escaping, and pass the result directly to qemuMonitorHMPCommandWithFd. If the monitor is in json mode, these pre-escaped commands will be escaped again when converted to json, which can result in the wrong arguments being sent. For example, a filename test\file would be sent in json as test\\file. This prevented attaching an image file with a or \ in its name in qemu 1.0.50, and also broke rbd attachment (which uses backslashes to escape some internal arguments.) Reported-by: Masuko Tomoya tomoya.mas...@gmail.com Signed-off-by: Josh Durgin josh.dur...@dreamhost.com --- .gitignore |1 + src/qemu/qemu_monitor.c | 59 +++- src/qemu/qemu_monitor.h |1 + tests/Makefile.am | 12 - tests/qemumonitortest.c | 114 +++ 5 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 tests/qemumonitortest.c diff --git a/.gitignore b/.gitignore index b7561dc..264a419 100644 --- a/.gitignore +++ b/.gitignore @@ -128,6 +128,7 @@ /tests/openvzutilstest /tests/qemuargv2xmltest /tests/qemuhelptest +/tests/qemumonitortest /tests/qemuxmlnstest /tests/qparamtest /tests/reconnect diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 93f3505..85212a7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -153,6 +153,49 @@ char *qemuMonitorEscapeArg(const char *in) return out; } +char *qemuMonitorUnescapeArg(const char *in) +{ +int i, j; +char *out; +int len = strlen(in) + 1; +char next; + +if (VIR_ALLOC_N(out, len) 0) +return NULL; + +for (i = j = 0; i len; ++i) { +next = in[i]; +if (in[i] == '\\') { +if (len i + 1) { +// trailing backslash shouldn't be possible +VIR_FREE(out); +return NULL; +} +++i; +switch(in[i]) { +case 'r': +next = '\r'; +break; +case 'n': +next = '\n'; +break; +case '': +case '\\': +next = in[i]; +break; +default: +// invalid input +VIR_FREE(out); +return NULL; +} +} +out[j++] = next; +} +out[j] = '\0'; + +return out; +} + #if DEBUG_RAW_IO # include c-ctype.h static char * qemuMonitorEscapeNonPrintable(const char *text) @@ -852,10 +895,20 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, int scm_fd, char **reply) { -if (mon-json) -return qemuMonitorJSONHumanCommandWithFd(mon, cmd, scm_fd, reply); -else +char *json_cmd = NULL; +if (mon-json) { +// hack to avoid complicating each call to text monitor functions +json_cmd = qemuMonitorUnescapeArg(cmd); +if (!json_cmd) { +VIR_DEBUG(Could not unescape command: %s, cmd); +qemuReportError(VIR_ERR_INTERNAL_ERROR, %s, +_(Unable to unescape command)); +return -1; +} +return qemuMonitorJSONHumanCommandWithFd(mon, json_cmd, scm_fd, reply); +} else { return qemuMonitorTextCommandWithFd(mon, cmd, scm_fd, reply); +} } /* Ensure proper locking around callbacks. */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7c6c52b..9768457 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -128,6 +128,7 @@ struct _qemuMonitorCallbacks { char *qemuMonitorEscapeArg(const char *in); +char *qemuMonitorUnescapeArg(const char *in); qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, virDomainChrSourceDefPtr config, diff --git a/tests/Makefile.am b/tests/Makefile.am index 9974c2f..3e505a5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -72,6 +72,7 @@ EXTRA_DIST = \ nwfilterxml2xmlout \ oomtrace.pl \ qemuhelpdata \ + qemumonitortest \ qemuxml2argvdata \ qemuxml2xmloutdata \ qemuxmlnsdata \ @@ -110,7 +111,8 @@ check_PROGRAMS += xml2sexprtest sexpr2xmltest \ endif if WITH_QEMU check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ - qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest + qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ + qemumonitortest endif if WITH_OPENVZ @@ -237,7 +239,8 @@ endif if WITH_QEMU TESTS += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest qemuargv2xmltest \ -qemuhelptest domainsnapshotxml2xmltest nwfilterxml2xmltest +qemuhelptest domainsnapshotxml2xmltest
[libvirt] Add GSoC project ideas to the wiki!
This is a reminder that QEMU will apply for Google Summer of Code 2012 and we need project ideas and mentors. Libvirt and kvm.ko projects are also welcome! http://wiki.qemu.org/Google_Summer_of_Code_2012 Please add yourself to the wiki now if you want to mentor a project this summer. I will file our application next week. Thanks, Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] Support for cpu64-rhel* qemu cpu models
In qemu there are 2 cpu models (cpu64-rhel5 and cpu64-rhel6) not supported by libvirt. This patch adds the support with the flags specifications from /usr/share/qemu-kvm/cpu-model/cpu-x86_64.conf The only difference is that AMD-specific features are removed so the processor type is not vendor-specific. --- v4: - removed AMD-specific features (3dnow, 3dnowext, svm) v3: - fixed sse3 naming (it's 'pni' in the features) v2: - removed duplicated entries src/cpu/cpu_map.xml | 60 +++ 1 files changed, 60 insertions(+), 0 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 2053f96..da3db02 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -309,6 +309,66 @@ feature name='pni'/ /model +model name='cpu64-rhel5' + feature name='apic'/ + feature name='clflush'/ + feature name='cmov'/ + feature name='cx8'/ + feature name='de'/ + feature name='fpu'/ + feature name='fxsr'/ + feature name='lm'/ + feature name='mca'/ + feature name='mce'/ + feature name='mmx'/ + feature name='msr'/ + feature name='mtrr'/ + feature name='nx'/ + feature name='pae'/ + feature name='pat'/ + feature name='pge'/ + feature name='pse'/ + feature name='pse36'/ + feature name='sep'/ + feature name='sse'/ + feature name='sse2'/ + feature name='pni'/ + feature name='syscall'/ + feature name='tsc'/ +/model + +model name='cpu64-rhel6' + feature name='abm'/ + feature name='apic'/ + feature name='clflush'/ + feature name='cmov'/ + feature name='cx16'/ + feature name='cx8'/ + feature name='de'/ + feature name='fpu'/ + feature name='fxsr'/ + feature name='lahf_lm'/ + feature name='lm'/ + feature name='mca'/ + feature name='mce'/ + feature name='mmx'/ + feature name='msr'/ + feature name='mtrr'/ + feature name='nx'/ + feature name='pae'/ + feature name='pat'/ + feature name='pge'/ + feature name='pse'/ + feature name='pse36'/ + feature name='sep'/ + feature name='sse'/ + feature name='sse2'/ + feature name='pni'/ + feature name='sse4a'/ + feature name='syscall'/ + feature name='tsc'/ +/model + model name='kvm32' model name='pentiumpro'/ feature name='mtrr'/ -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0
It's possible to disable SPICE TLS in qemu.conf. When this happens, libvirt ignores any SPICE TLS port or x509 directory that may have been set when it builds the qemu command line to use. However, it's not ignoring the secure channels that may have been set and adds tls-channel arguments to qemu command line. Current qemu versions don't report an error when this happens, and try to use TLS for the specified channels. Before this patch domain type='kvm' nameauto-tls-port/name memory65536/memory os type arch='x86_64' machine='pc'hvm/type /os devices graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' ke listen type='address' address='0'/ channel name='main' mode='secure'/ channel name='inputs' mode='secure'/ /graphics /devices /domain generates -spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs and starts QEMU. After this patch, an error is reported if a TLS port is set in the XML or if secure channels are specified but TLS is disabled in qemu.conf. This is the behaviour the oVirt people (where I spotted this issue) said they would expect. This fixes bug #790436 --- src/qemu/qemu_command.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a34504..4f3e61e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAsprintf(opt, port=%u, def-graphics[0]-data.spice.port); -if (driver-spiceTLS def-graphics[0]-data.spice.tlsPort != -1) +if (def-graphics[0]-data.spice.tlsPort != -1) +if (!driver-spiceTLS) { +qemuReportError(VIR_ERR_XML_ERROR, +_(spice TLS port set in XML configuration, but TLS is disabled in qemu.conf)); +goto error; +} virBufferAsprintf(opt, ,tls-port=%u, def-graphics[0]-data.spice.tlsPort); switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) { @@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn, int mode = def-graphics[0]-data.spice.channels[i]; switch (mode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: +if (!driver-spiceTLS) { +qemuReportError(VIR_ERR_XML_ERROR, +_(spice secure channels set in XML configuration, but TLS is disabled in qemu.conf)); +goto error; +} virBufferAsprintf(opt, ,tls-channel=%s, virDomainGraphicsSpiceChannelNameTypeToString(i)); break; -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add GSoC project ideas to the wiki!
On 24.02.2012 10:19, Stefan Hajnoczi wrote: This is a reminder that QEMU will apply for Google Summer of Code 2012 and we need project ideas and mentors. Libvirt and kvm.ko projects are also welcome! http://wiki.qemu.org/Google_Summer_of_Code_2012 Please add yourself to the wiki now if you want to mentor a project this summer. I will file our application next week. Thanks, Stefan Hi Stefan, Thank you for the opportunity. I was personally thinking about something libvirt-snmp related. Nowdays, it is difficult to add new elements to MIB, as some parts of code were generated by mib2c. Any change to MIB requires regeneration of such source files and thus leads to loss of all previous changes. So one thing that is coming to my mind is drop this dependency and use libsnmp directly. But I am not sure it is worth of GSoC. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Error out when using SPICE TLS with spice_tls=0
On Fri, Feb 24, 2012 at 11:34:45AM +0100, Christophe Fergeau wrote: It's possible to disable SPICE TLS in qemu.conf. When this happens, libvirt ignores any SPICE TLS port or x509 directory that may have been set when it builds the qemu command line to use. However, it's not ignoring the secure channels that may have been set and adds tls-channel arguments to qemu command line. Current qemu versions don't report an error when this happens, and try to use TLS for the specified channels. Before this patch domain type='kvm' nameauto-tls-port/name memory65536/memory os type arch='x86_64' machine='pc'hvm/type /os devices graphics type='spice' port='5900' tlsPort='-1' autoport='yes' listen='0' ke listen type='address' address='0'/ channel name='main' mode='secure'/ channel name='inputs' mode='secure'/ /graphics /devices /domain generates -spice port=5900,addr=0,disable-ticketing,tls-channel=main,tls-channel=inputs and starts QEMU. After this patch, an error is reported if a TLS port is set in the XML or if secure channels are specified but TLS is disabled in qemu.conf. This is the behaviour the oVirt people (where I spotted this issue) said they would expect. This fixes bug #790436 --- src/qemu/qemu_command.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a34504..4f3e61e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5231,7 +5231,12 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAsprintf(opt, port=%u, def-graphics[0]-data.spice.port); -if (driver-spiceTLS def-graphics[0]-data.spice.tlsPort != -1) +if (def-graphics[0]-data.spice.tlsPort != -1) +if (!driver-spiceTLS) { +qemuReportError(VIR_ERR_XML_ERROR, +_(spice TLS port set in XML configuration, but TLS is disabled in qemu.conf)); +goto error; +} virBufferAsprintf(opt, ,tls-port=%u, def-graphics[0]-data.spice.tlsPort); switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) { @@ -5287,6 +5292,11 @@ qemuBuildCommandLine(virConnectPtr conn, int mode = def-graphics[0]-data.spice.channels[i]; switch (mode) { case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: +if (!driver-spiceTLS) { +qemuReportError(VIR_ERR_XML_ERROR, +_(spice secure channels set in XML configuration, but TLS is disabled in qemu.conf)); +goto error; +} virBufferAsprintf(opt, ,tls-channel=%s, virDomainGraphicsSpiceChannelNameTypeToString(i)); break; ACK, if we s/XML_ERROR/CONFIG_UNSUPPORTED/ in the those two error messages 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] Fixed service handling in specfile
After adding the libvirt-guests service into usual runlevels, we used to start the libvirt-guests service. However this is usually not a good practice. As mentioned on fedoraproject wiki, the installations can be in changeroots, in an installer context, or in other situations where we don't want the services autostarted. --- libvirt.spec.in |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 67cde23..072fd8e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1027,14 +1027,6 @@ fi %if %{with_systemd} %else /sbin/chkconfig --add libvirt-guests -if [ $1 -ge 1 ]; then -level=$(/sbin/runlevel | /bin/cut -d ' ' -f 2) -if /sbin/chkconfig --levels $level libvirt-guests; then -# this doesn't do anything but allowing for libvirt-guests to be -# stopped on the first shutdown -/sbin/service libvirt-guests start /dev/null 21 || true -fi -fi %endif %postun client -p /sbin/ldconfig -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt TCK wrapper for autotest review
On 02/23/2012 12:27 PM, Guannan Ren wrote: Hi Lucas, Thanks for your these good modifications. There is one place I noticed where you output each testcase of *.t into a separate file with .tap extension. hence, it has a corresponding log file with little content for each testcase. it seem a little harder to check compared to just one log file. The rest of them is perfect for me. Guannan Ren Thanks! Now, feel free to pick this up and modify as you wish, then send me as a pull request/patch to the mailing list. I'll close the example pull request and will be waiting on you, ok? Cheers, Lucas -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch v2] vmx: Better Workstation vmx handling
2012/2/23 Jean-Baptiste Rouault jean-baptiste.roua...@diateam.net: This patch adds support for vmx files with empty networkName values (which is the case for vmx generated by Workstation). It also adds support for vmx containing NATed network interfaces. Update test suite accordingly --- ACK and pushed, thanks. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add GSoC project ideas to the wiki!
On Fri, Feb 24, 2012 at 10:38 AM, Michal Privoznik mpriv...@redhat.com wrote: On 24.02.2012 10:19, Stefan Hajnoczi wrote: Thank you for the opportunity. I was personally thinking about something libvirt-snmp related. Nowdays, it is difficult to add new elements to MIB, as some parts of code were generated by mib2c. Any change to MIB requires regeneration of such source files and thus leads to loss of all previous changes. So one thing that is coming to my mind is drop this dependency and use libsnmp directly. But I am not sure it is worth of GSoC. If this project is self-contained and can be completed in 12 weeks by a person without prior libvirt and SNMP experience, but fluent C programming skills, then it sounds suitable. Is there any creativity required or problems to solve that aren't grunt work? For example, if you just need to run mib2c and then manually diff to produce the final C version, then this sounds like a lot of manual work but little gain for the student. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Post-install behavior of libvirt(-client)
On 02/23/2012 04:23 PM, Eric Blake wrote: On 02/23/2012 07:25 AM, Martin Kletzander wrote: To me it seems more reasonable to just don't start anything. I don't want any service on my system started just because I installed it. I think we've reached a state of violent agreement :) I didn't mean it that way but I'm happy we came to an agreement :) So to summarize it. Please tell me if I can remove the parts of the %post script dealing with starting services. Yes, and I will gladly ACK such patches. Patch is sent (it was starting the service on only one place, so it's really small one). Thanks for tackling this, and for making me do further research onto Fedora packaging guidelines. And thank you for having such patience with me :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Fixed URI parsing
Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. File changes: - src/util/xml.h -- declaration - src/util/xml.c -- definition - src/libvirt_private.syms -- symbol export - all others -- function call fixed and include added --- v2: - added virSaveURI for building back the original string src/esx/esx_driver.c |3 +- src/libvirt.c |7 ++- src/libvirt_private.syms |2 + src/libxl/libxl_driver.c |3 +- src/lxc/lxc_driver.c |3 +- src/openvz/openvz_driver.c |3 +- src/qemu/qemu_driver.c |2 +- src/qemu/qemu_migration.c |5 +- src/remote/remote_driver.c |5 +- src/uml/uml_driver.c |3 +- src/util/xml.c | 94 src/util/xml.h |5 ++ src/vbox/vbox_tmpl.c |3 +- src/vmx/vmx.c |3 +- src/xen/xen_driver.c |2 +- src/xen/xend_internal.c|3 +- 16 files changed, 129 insertions(+), 17 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index f5e1cc7..4375432 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -44,6 +44,7 @@ #include esx_vi.h #include esx_vi_methods.h #include esx_util.h +#include xml.h #define VIR_FROM_THIS VIR_FROM_ESX @@ -3976,7 +3977,7 @@ esxDomainMigratePerform(virDomainPtr domain, } /* Parse migration URI */ -parsedUri = xmlParseURI(uri); +parsedUri = virParseURI(uri); if (parsedUri == NULL) { virReportOOMError(); diff --git a/src/libvirt.c b/src/libvirt.c index 6294196..465d0aa 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -44,6 +44,7 @@ #include command.h #include virnodesuspend.h #include virrandom.h +#include xml.h #ifndef WITH_DRIVER_MODULES # ifdef WITH_TEST @@ -1127,7 +1128,7 @@ do_open (const char *name, virConnectOpenResolveURIAlias(name, alias) 0) goto failed; -ret-uri = xmlParseURI (alias ? alias : name); +ret-uri = virParseURI (alias ? alias : name); if (!ret-uri) { virLibConnError(VIR_ERR_INVALID_ARG, _(could not parse connection URI %s), @@ -1729,7 +1730,7 @@ virConnectGetURI (virConnectPtr conn) return NULL; } -name = (char *)xmlSaveUri(conn-uri); +name = (char *)virSaveURI(conn-uri); if (!name) { virReportOOMError(); goto error; @@ -4964,7 +4965,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain, return -1; } -tempuri = xmlParseURI(dconnuri); +tempuri = virParseURI(dconnuri); if (!tempuri) { virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__); virDispatchError(domain-conn); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e3573f..595020a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1431,6 +1431,8 @@ virTypedParameterAssign; # xml.h +virParseURI; +virSaveURI; virXMLChildElementCount; virXMLParseHelper; virXMLPropString; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6cfc5eb..f1ef5fb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -44,6 +44,7 @@ #include libxl_conf.h #include xen_xm.h #include virtypedparam.h +#include xml.h #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -1043,7 +1044,7 @@ libxlOpen(virConnectPtr conn, if (libxl_driver == NULL) return VIR_DRV_OPEN_DECLINED; -conn-uri = xmlParseURI(xen:///); +conn-uri = virParseURI(xen:///); if (!conn-uri) { virReportOOMError(); return VIR_DRV_OPEN_ERROR; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b6962cf7..3d25d5e 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -60,6 +60,7 @@ #include virnodesuspend.h #include virtime.h #include virtypedparam.h +#include xml.h #define VIR_FROM_THIS VIR_FROM_LXC @@ -139,7 +140,7 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn, if (lxc_driver == NULL) return VIR_DRV_OPEN_DECLINED; -conn-uri = xmlParseURI(lxc:///); +conn-uri = virParseURI(lxc:///); if (!conn-uri) { virReportOOMError(); return VIR_DRV_OPEN_ERROR; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 833a98d..47fad74 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -56,6 +56,7 @@ #include virfile.h #include logging.h #include command.h +#include xml.h #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -1335,7
Re: [libvirt] [PATCH v2] Fixed URI parsing
On Fri, Feb 24, 2012 at 02:30:11PM +0100, Martin Kletzander wrote: Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. diff --git a/src/util/xml.h b/src/util/xml.h index a3750fa..4835900 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -10,6 +10,7 @@ # include libxml/parser.h # include libxml/tree.h # include libxml/xpath.h +# include libxml/uri.h int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); @@ -61,6 +62,10 @@ xmlDocPtr virXMLParseHelper(int domcode, const char *url, xmlXPathContextPtr *pctxt); +xmlURIPtrvirParseURI(const char *uri); + +unsigned char * virSaveURI(xmlURIPtr uri); + Can you create new files for these 'util/viruri.{h,c}' and change to ensure a standard 'virURI' naming prefix. Also we tend to use the pair Parse/Format, rather than Parse/Save in libvirt code. So eg create a file viruri.h containing: typedef virURIPtr xmlURIPtr; virURIPtr virURIParse(const char *uri); char * virURIFormat(virURIPtr uri); 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 v2] Fixed URI parsing
On 02/24/2012 06:30 AM, Martin Kletzander wrote: Function xmlParseURI does not remove square brackets around IPv6 address when parsing. One of the solutions is making wrappers around functions working with xmlURI*. This assures that uri-server will be always properly assigned and it doesn't have to be changed when used on some new place in the code. For this purpose, functions virParseURI and virSaveURI were added. These function are wrappers around xmlParseURI and xmlSaveUri respectively. + */ +unsigned char * +virSaveURI(xmlURIPtr uri) +{ +char *tmpserver, *backupserver = uri-server; +unsigned char *ret; +bool fixback = false; Instead of using bool fixback, I'd set tmpserver as NULL here... + +/* First check: does it make sense to do anything */ +if (uri != NULL +uri-server != NULL +strchr(uri-server, ':') != NULL) { + +/* To be sure we have enough space for the brackets, we save + * the old string and then alocate a new one */ s/alocate/allocate/ - but see below, as I don't think you need this comment. + + /* the + 2 is room for square brackets and + 1 for '\0' */ +size_t length = strlen(uri-server) + 3; + +if(VIR_ALLOC_N(tmpserver, length) 0) { +virReportOOMError(); No need to raise OOM error here - all callers of xmlSaveUri were already raising their own OOM after a NULL return. +return NULL; +} + +snprintf(tmpserver, length, [%s], uri-server); I'd just use virAsnprintf(tmpserver, [%s], uri-server); none of this need to call strlen, VIR_ALLOC_N, or snprintf. + +uri-server = tmpserver; +bool fixback = true; +} + +ret = xmlSaveUri(uri); + +/* Put the fixed version back */ +if (fixback) { ...and check 'if (tmpserver)' here (that is, fixback is redundant with whether tmpserver was assigned at this point). +uri-server = backupserver; +VIR_FREE(tmpserver); +} + +return ret; +} I'm also a bit worried that we might regress if we don't add a syntax check; can you look at adding a rule to cfg.mk that poisons xmlParseURI and xmlSaveUri (wow, libxml2 really does have the awful URI vs. Uri in its naming conventions), then exempt util/uri.c from the check as the only file allowed to use them. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list