Re: [libvirt] [PATCH 0/5] remote generator: Cover more functions
On Sun, May 15, 2011 at 08:23:29AM +0200, Matthias Bolte wrote: Move more functions to the generator and cover stream-using functions. As the stream-usage is not detectable from the .x file directly add additional annotations. daemon/remote.c | 185 --- daemon/remote_generator.pl | 211 +++ daemon/stream.c |7 +- src/driver.h |2 +- src/libvirt.c|4 +- src/lxc/lxc_driver.c |2 +- src/qemu/qemu_driver.c |2 +- src/remote/remote_driver.c | 288 -- src/remote/remote_protocol.x | 26 +++-- src/uml/uml_driver.c |2 +- src/xen/xen_driver.c |2 +- 11 files changed, 188 insertions(+), 543 deletions(-) I went through the patch set and didn't detected anything suspicious, but it would be better if someone knowing the remote stubs better actually gave the final ACK. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FreeBSD and sysconf(_SC_GETPW_R_SIZE_MAX)
On Sun, May 15, 2011 at 07:22:46AM +0200, Matthias Bolte wrote: On FreeBSD virsh fails to enter interactive mode because vshReadlineInit fails, because virGetUserDirectory fails, because virGetUserEnt fails, because sysconf(_SC_GETPW_R_SIZE_MAX) returns -1 and sets errno to EINVAL. Is this something that gnulib should/could deal with, Eric? Or should we work around it in libvrt and fallback to PATH_MAX when sysconf(_SC_GETPW_R_SIZE_MAX) fails? Hum, I'm also seeing a number of use of sysconf for _SC_GETGR_R_SIZE_MAX which also isn't listed in my linux man page as one of the possible values, so I'm wondering how portable this actually is ... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FreeBSD and sys/syslimits.h
On Sat, May 14, 2011 at 06:49:40PM +0200, Matthias Bolte wrote: libvirtd.h includes sys/syslimits.h if available. On FreeBSD the header contains this warning: In file included from libvirtd.h:42, from dispatch.h:28, from dispatch.c:30: /usr/include/sys/syslimits.h:41:2: warning: #warning No user-serviceable parts inside. Here's the relevant part from that header: #if !defined(_KERNEL) !defined(_LIMITS_H_) !defined(_SYS_PARAM_H_) #ifndef _SYS_CDEFS_H_ #error this file needs sys/cdefs.h as a prerequisite #endif #ifdef __CC_SUPPORTS_WARNING #warning No user-serviceable parts inside. #endif #endif This looks like this header is not meant to be included directly, at least on FreeBSD. And on my linux box syslimits.h seems to come from gcc, verbatim: - /* syslimits.h stands for the system's own limits.h file. If we can use it ok unmodified, then we install this text. If fixincludes fixes it, then the fixed version is installed instead of this text. */ #define _GCC_NEXT_LIMITS_H /* tell gcc's limits.h to recurse */ #include_next limits.h #undef _GCC_NEXT_LIMITS_H - Shouldn't we just include limits.h directly ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Improve editing
On Fri, May 13, 2011 at 02:42:29PM +0100, Daniel P. Berrange wrote: There are two common problems with virsh edit friends - Invalid XML syntax, causes error report lost changes - User add unsupported/unknown XML attributes/elements which are silently discarded by libvirt This patch only fixes the first problem. It would be nice to fix the second two, by running the XML through the RNG schema validator. Rather than do this in virsh though, I'd add some flags to the virDefine/Create APIs, eg VIR_DOMAIN_XML_VALIDATE virsh can set this flag by default, and if the XML fails validation, it could prompt the user, asking if they want to proceed anyway (in which case recall the same API but without the validate flag set), or re-edit the XML Hum, yes I agree with the option of validating on define of APIs the only problem is that we tend to have holes in the RNG, but since that would be optional I think that's okay, this would hopefully help finding the mismatches between the RNG and the C parsing code. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Allow hvsupport.html.in to be auto-generated
On Fri, May 13, 2011 at 05:31:54PM -0600, Eric Blake wrote: On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is constantly out of date, because when updating the drivers to add new APIs, people often (always) forget to update the hvsupport.html.in file. To solve this we can instead store version number annotations in the drivers themselves, so it is not easily missed. Then the hvsupport.html.in file can be auto-generated I love the idea! But I ran out of time to finish reviewing it today. Did you test VPATH builds? (autogen.sh will help). Did you test 'make distcheck' to ensure the tarball is complete? I like the idea too, except for the perl part, does it make it mandatory to have perl to build now (the remote generator may have pushed that dependancy already though). I would actually put patch 4 first since it's a no-op and then patch 3 since it depends on 4 for the genration (but it's a detail). Checking the full patch 4 seems hard did you just diff the current version and generated one (I would guess so) ? If yes ACK from me after checking builds as Eric suggests, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 5/8] screenshot: Expose the new API in virsh
On Fri, May 13, 2011 at 05:18:27PM -0600, Eric Blake wrote:
On 05/13/2011 03:16 AM, Daniel P. Berrange wrote:
On Thu, May 12, 2011 at 06:29:12PM +0200, Michal Privoznik wrote:
+static const vshCmdOptDef opts_screenshot[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{file, VSH_OT_DATA, VSH_OFLAG_REQ, N_(where to store the
screenshot)},
+{screen, VSH_OT_INT, 0, N_(ID of a screen to take screenshot of)},
+{NULL, 0, 0, NULL}
+};
+
I think it would be better if 'screenID' was a flag instead of a
positional parameter, eg
screenshot --screen 1 myguest imagefile.png
But virsh _already_ handles all named arguments in any order. That is,
screenshot --screen 1 myguest imagefile.png
screenshot myguest imagefile.png 1
are identical, at least with the above opts_screenshot (thanks to commit
b9973f5).
I don't see how that could work, if you want to make filename
optional. Since if you have
screenshot myguest 1
it can't determine whether '1' is a screen number or filename. Thus
we have to use --screen for this
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 v4 0/8] Add support for taking screenshots of domain console
On Fri, May 13, 2011 at 05:15:56PM -0600, Eric Blake wrote: On 05/13/2011 02:56 AM, Daniel P. Berrange wrote: On Fri, May 13, 2011 at 10:34:24AM +0200, Michal Prívozník wrote: Then, screen is calculated as Screen Device Head 0 video0 0 1 video0 1 2 video1 0 3 video1 1 4 video1 2 5 video1 3 Incidentally an RFE is needed against QEMU, since it can only do screen dump of the first device :-( Daniel Yes, that is what I had in my mind when creating this concept. Ok, please document that in the API docs for the public API Or would it be better to split screen ID into video # and head #? No, I think that's probably overkill, unless anyone can think of something we can do with them separated, that we can't do with them combined... Good thing we have the flags argument. When you have a card that supports multiple monitors, I could see it being worth capturing a screenshot of just one monitor, vs. a combined screenshot of both monitors as a single image. With the above layout, this could be done as: virDomainScreenshot(dom, st, 1, 0) - just screen 1 (head 1 of video0) virDomainScreenshot(dom, st, 1, VIR_DOMAIN_SCREENSHOT_DEVICE) - composite of all screens on video1 (that is, screens 2-5) of course, supposing that the hypervisors support combined imaging. The only way I could see that being possible, is if there was actually a guest OS agent that captured the screenshot, because the hypervisor has no way of knowing what the layout of the screens are. ie if the device has 2 outputs and you want a single image of both outputs how do you know if the output 1 is left of output 2, or right of it, or above it, or below it, etc. 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] FreeBSD and sysconf(_SC_GETPW_R_SIZE_MAX)
On Sun, May 15, 2011 at 07:22:46AM +0200, Matthias Bolte wrote:
On FreeBSD virsh fails to enter interactive mode because
vshReadlineInit fails, because virGetUserDirectory fails, because
virGetUserEnt fails, because sysconf(_SC_GETPW_R_SIZE_MAX) returns -1
and sets errno to EINVAL.
Is this something that gnulib should/could deal with, Eric?
Or should we work around it in libvrt and fallback to PATH_MAX when
sysconf(_SC_GETPW_R_SIZE_MAX) fails?
PATH_MAX isn't expected to be available on all platforms either which
is one of the reasons for us removing its use.
getpwnam() returns ERANGE if the allocated buffer is too small. So
we should do something along the lines of
size_t len = sysconf(_SC_GETPW_R_SIZE_MAX)
if (len 0)
len = 1024;
while (1) {
...
err = getpwnam()
if (err 0) {
if (errno == ERANGE) {
len += 1024;
continue;
}
}
}
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] domain: Require init for container guests
On Fri, May 13, 2011 at 10:46:51AM -0400, Cole Robinson wrote: On 05/13/2011 05:27 AM, Daniel P. Berrange wrote: On Thu, May 12, 2011 at 05:47:42PM -0400, Cole Robinson wrote: Use capabilities to allow a driver to register a default init if none is specified in the XML. Openvz was already open-coding this to be /sbin/init LXC currently falls over if no init is specified, so an explicit error is an improvement IMO. (Side note: I don't think we can set a default value for LXC. If we use /sbin/init but the user doesn't specify a separate root FS for their guest, the container will rerun the host's init which can be traumatic :). For virt-install I'm thinking of defaulting to /sbin/init if a root FS has been specified, otherwise require the user to manually specify init) We could set '/bin/sh' as the default for non-root FS containers. In virt-install or at the libvirt level? I'd do it at the libvirt level 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] Allow hvsupport.html.in to be auto-generated
On Fri, May 13, 2011 at 02:36:55PM +0100, Daniel P. Berrange wrote:
The hvsupport.html.in file is constantly out of date, because when
updating the drivers to add new APIs, people often (always) forget
to update the hvsupport.html.in file.
To solve this we can instead store version number annotations in
the drivers themselves, so it is not easily missed. Then the
hvsupport.html.in file can be auto-generated
In libguestfs we generate the equivalent information entirely
automatically. See:
http://git.annexia.org/?p=libguestfs.git;a=tree;f=src/api-support;hb=HEAD
http://git.annexia.org/?p=libguestfs.git;a=blob;f=src/api-support/update-from-tarballs.sh;hb=HEAD
It requires access to a directory containing all the
{libguestfs|libvirt} tarballs from every released version.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Allow hvsupport.html.in to be auto-generated
On Mon, May 16, 2011 at 10:41:52AM +0100, Richard W.M. Jones wrote:
On Fri, May 13, 2011 at 02:36:55PM +0100, Daniel P. Berrange wrote:
The hvsupport.html.in file is constantly out of date, because when
updating the drivers to add new APIs, people often (always) forget
to update the hvsupport.html.in file.
To solve this we can instead store version number annotations in
the drivers themselves, so it is not easily missed. Then the
hvsupport.html.in file can be auto-generated
In libguestfs we generate the equivalent information entirely
automatically. See:
http://git.annexia.org/?p=libguestfs.git;a=tree;f=src/api-support;hb=HEAD
http://git.annexia.org/?p=libguestfs.git;a=blob;f=src/api-support/update-from-tarballs.sh;hb=HEAD
It requires access to a directory containing all the
{libguestfs|libvirt} tarballs from every released version.
That is just providing a list of public APIs per version. We already
generate that information from the src/libvirt_public.syms file which
has the public API versioning script.
The annotations are required in order to specify what each internal
driver implementation supports. Extracting this information from
the historical tarballs is too complex, unless we actually ran a
proper C parser over each codebase to extract the internal structs
and methods used. In the end, just annotating the drivers is far
simpler, than writing an even more complex script.
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 v4 5/8] screenshot: Expose the new API in virsh
On 16.05.2011 10:56, Daniel P. Berrange wrote:
On Fri, May 13, 2011 at 05:18:27PM -0600, Eric Blake wrote:
On 05/13/2011 03:16 AM, Daniel P. Berrange wrote:
On Thu, May 12, 2011 at 06:29:12PM +0200, Michal Privoznik wrote:
+static const vshCmdOptDef opts_screenshot[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{file, VSH_OT_DATA, VSH_OFLAG_REQ, N_(where to store the
screenshot)},
+{screen, VSH_OT_INT, 0, N_(ID of a screen to take screenshot of)},
+{NULL, 0, 0, NULL}
+};
+
I think it would be better if 'screenID' was a flag instead of a
positional parameter, eg
screenshot --screen 1 myguest imagefile.png
But virsh _already_ handles all named arguments in any order. That is,
screenshot --screen 1 myguest imagefile.png
screenshot myguest imagefile.png 1
are identical, at least with the above opts_screenshot (thanks to commit
b9973f5).
I don't see how that could work, if you want to make filename
optional. Since if you have
screenshot myguest 1
This is now understand as: take screenshot of myguest, screenID=0 (by
default) and store it into file named '1'.
Running 'screenshot --screen 1 f14' would produce an error: file
required. I agree not to make arguments positional, and basically
screenID is not. It is an added value that it can be specified as 3rd
argument.
If we make --file optional (and generate it), this will still work as
expected: 'screenshot --screen 1 f14' would not produce any error but
file instead. Moreover, screenshot f14 would take screenshot of
screenID=0 and store it into file with generated file.
it can't determine whether '1' is a screen number or filename. Thus
we have to use --screen for this
Daniel
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Allow hvsupport.html.in to be auto-generated
On Mon, May 16, 2011 at 03:19:37PM +0800, Daniel Veillard wrote: On Fri, May 13, 2011 at 05:31:54PM -0600, Eric Blake wrote: On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is constantly out of date, because when updating the drivers to add new APIs, people often (always) forget to update the hvsupport.html.in file. To solve this we can instead store version number annotations in the drivers themselves, so it is not easily missed. Then the hvsupport.html.in file can be auto-generated I love the idea! But I ran out of time to finish reviewing it today. Did you test VPATH builds? (autogen.sh will help). Did you test 'make distcheck' to ensure the tarball is complete? I like the idea too, except for the perl part, does it make it mandatory to have perl to build now (the remote generator may have pushed that dependancy already though). Perl is already mandatory for people building from GIT, so this doesn't change that requirement at all. I would actually put patch 4 first since it's a no-op and then patch 3 since it depends on 4 for the genration (but it's a detail). Yep, I've reordered this. Checking the full patch 4 seems hard did you just diff the current version and generated one (I would guess so) ? If yes ACK from me after checking builds as Eric suggests, The current generated one is too incomplete to be useful for comparison. I've basically worked on each driver in turn. Starting with APIs present when the driver was introduced, then viewing a git diff between subsequent releases tags, to identify what APIs were added in each release. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/9] Introduce virDomainGetState API
On Tue, May 10, 2011 at 15:39:02 +0200, Jiri Denemark wrote: This new API solves several problems: - calling virDomainGetInfo for just getting domain status is an overkill since it may result in sending requests to guest OS - since virDomainGetInfo can hang when guest OS is not responding and it is used by virsh list, listing domains can hang - virDomainGetState provides additional info about what action led to domain's current state, which can be used instead of listening to domain events Version 2: - rebased to current HEAD - unsigned int flags parameter - updated version info in public.syms - noGetState renamed as useGetInfo in virsh.c - simplified implementation in esx driver per Matthias' suggestion - call internal xen drivers directly instead of going through xenUnifiedDriver - fixed || vs typo in domain_conf.c - new patch 9/9: qemu: Update domain state when reconnecting monitor I pushed this series now. Thanks for the reviews. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] spice: support streaming-video parameter
This adds a streaming-video=filter|all|off attribute. It is used to change
the behavior of video stream detection in spice, the default is filter (the
default for libvirt is not to specify it - the actual default is defined in
libspice-server.so).
Usage:
graphics type='spice' autoport='yes'
streaming mode='off'/
/graphics
Tested with the above and with tests/qemuxml2argvtest.
Signed-off-by: Alon Levy al...@redhat.com
---
Fixed: error code for missing mode is now VIR_ERR_XML_ERROR, and added
documentation.
Note: I'm not registered to the list, so please cc me on reply, thanks.
---
docs/formatdomain.html.in |6
docs/schemas/domain.rng| 12
src/conf/domain_conf.c | 30
src/conf/domain_conf.h | 11 +++
src/libvirt_private.syms |2 +
src/qemu/qemu_command.c|3 ++
.../qemuxml2argv-graphics-spice.args |2 +-
.../qemuxml2argv-graphics-spice.xml|1 +
8 files changed, 66 insertions(+), 1 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5013c48..78fbc09 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1775,6 +1775,7 @@ qemu-kvm -net nic,model=? /dev/null
lt;channel name='main' mode='secure'/gt;
lt;channel name='record' mode='insecure'/gt;
lt;image compression='auto_glz'/gt;
+lt;streaming mode='filter'/gt;
lt;/graphicsgt;/pre
p
Spice supports variable compression settings for audio,
@@ -1793,6 +1794,11 @@ qemu-kvm -net nic,model=? /dev/null
and codeplayback/code for enabling audio stream
compression (accepts codeon/code or codeoff/code).
/p
+p
+ Streaming mode is set by the codestreaming/code
+ element, settings it's codemode/code attribute to one
+ of codefilter/code,codeall/code or codeoff/code.
+/p
/dd
dtcoderdp/code/dt
dd
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 7163c6e..9083ff9 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1334,6 +1334,18 @@
empty/
/element
/optional
+optional
+ element name=streaming
+attribute name=mode
+ choice
+valuefilter/value
+valueall/value
+valueoff/value
+ /choice
+/attribute
+empty/
+ /element
+/optional
/interleave
/group
group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2a681d9..9be459c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -352,6 +352,13 @@ VIR_ENUM_IMPL(virDomainGraphicsSpicePlaybackCompression,
on,
off);
+VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode,
+ VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_LAST,
+ default,
+ filter,
+ all,
+ off);
+
VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
subsystem,
capabilities)
@@ -4082,6 +4089,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags)
{
VIR_FREE(compression);
def-data.spice.playback = compressionVal;
+} else if (xmlStrEqual(cur-name, BAD_CAST streaming)) {
+const char *mode = virXMLPropString(cur, mode);
+int modeVal;
+
+if (!mode) {
+virDomainReportError(VIR_ERR_XML_ERROR, %s,
+ _(spice streaming missing
mode));
+goto error;
+}
+if ((modeVal =
+
virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) = 0) {
+virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unknown spice streaming
mode));
+VIR_FREE(mode);
+goto error;
+
+}
+VIR_FREE(mode);
+
+def-data.spice.streaming = modeVal;
}
}
cur = cur-next;
@@ -7979,6 +8006,9 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
if (def-data.spice.playback)
virBufferVSprintf(buf, playback compression='%s'/\n,
virDomainGraphicsSpicePlaybackCompressionTypeToString(def-data.spice.playback));
+if (def-data.spice.streaming)
+virBufferVSprintf(buf, streaming mode='%s'/\n,
+
Re: [libvirt] [PATCH 03/16] Introduce migration cookies to QEMU driver
On Thu, May 12, 2011 at 01:17:21PM -0600, Eric Blake wrote:
On 05/12/2011 10:04 AM, Daniel P. Berrange wrote:
The migration protocol has support for a 'cookie' parameter which
is an opaque array of bytes as far as libvirt is concerned. Drivers
may use this for passing around arbitrary extra data they might
need during migration. The QEMU driver needs to do a few things:
+static int
+qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
+xmlXPathContextPtr ctxt,
+int flags ATTRIBUTE_UNUSED)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+char *tmp;
+
+/* We don't store the uuid, name, hostname, or hostuuid
+ * values. We just compare them to local data to do some
+ * sanity checking on migration operation
+ */
Should we do sanity checking that no unknown XML elements appear in the
cookie? And/or validate that flags contains no unexpected bits? That
is, should you insert virCheckFlags(0, -1) here, or in
qemuMigrationEatCookie? And rather than just using virXPathString to
probe whether a particular XML element is present, shouldn't you instead
do an iteration over all XML elements in order to detect unrecognized
elements?
Otherwise, what I'm afraid of is that the cookie eater (whether the
destination eating the cookie from Begin, or the source eating the
cookie from Prepare) will be running an earlier libvirt version than the
baker; if the baker added a mandatory flag to the cookie, but the eater
is unaware to look for that element and silently ignores it, then we
risk silent botching of migration.
Do we have enough infrastructure in place for source and destination to
agree on what features are mandatory vs. optional in the cookie, to
allow for omission of optional elements that would make migration nicer
but aren't fatal if left out? That is, a baker can always try a flag,
then retry without the flag, but retries can get expensive if there were
an alternative to first do a capability query to determine the common
subset of flags to use in the first place.
Well, the migration cookies were intended to be optional data,
which if omitted, don't result in bad stuff.
eg, if the graphics relocation data doesn't exist, the end user
will simply need to manually reconnect.
I think we do need some stricter checking on the lock state
data though, to validate that the same lock manager is
present on both sides.
@@ -342,6 +612,15 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_MIGRATED);
+
+if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
0) 0) {
+/* We could tear down the whole guest here, but
+ * cookie data is (so far) non-critical, so that
+ * seems a little harsh. We'll just warn for now.
+ */
+VIR_WARN(Unable to encode migration cookie);
+}
+
ret = 0;
endjob:
@@ -369,7 +648,7 @@ cleanup:
virDomainObjUnlock(vm);
if (event)
qemuDomainEventQueue(driver, event);
-qemuDriverUnlock(driver);
+qemuMigrationCookieFree(mig);
Umm, did you really intend to drop the qemuDriverUnlock line?
No, that's a merge rebase error
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] Allow hvsupport.html.in to be auto-generated
On Fri, May 13, 2011 at 05:31:54PM -0600, Eric Blake wrote: On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is constantly out of date, because when updating the drivers to add new APIs, people often (always) forget to update the hvsupport.html.in file. To solve this we can instead store version number annotations in the drivers themselves, so it is not easily missed. Then the hvsupport.html.in file can be auto-generated I love the idea! But I ran out of time to finish reviewing it today. Did you test VPATH builds? (autogen.sh will help). Did you test 'make distcheck' to ensure the tarball is complete? There was a small typo in the makefile rule wrt VPATH builds which I've not fixed. Everything is correct inside the tarball 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
[libvirt] [PATCH] Disable virCommandExec on Win32
Mingw execve() has a broken signature. Disable this
function until gnulib fixes the signature, since we
don't really need this on Win32 anyway.
* src/util/command.c: Disable virCommandExec on Win32
---
src/util/command.c | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index 7ac411b..ebb90cb 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -989,6 +989,7 @@ cleanup:
* Returns -1 on any error executing the command.
* Will not return on success.
*/
+#ifndef WIN32
int virCommandExec(virCommandPtr cmd)
{
if (!cmd ||cmd-has_error == ENOMEM) {
@@ -1003,6 +1004,18 @@ int virCommandExec(virCommandPtr cmd)
return execve(cmd-args[0], cmd-args, cmd-env);
}
+#else
+int virCommandExec(virCommandPtr cmd ATTRIBUTE_UNUSED)
+{
+/* Mingw execve() has a broken signature. Disable this
+ * function until gnulib fixes the signature, since we
+ * don't really need this on Win32 anyway.
+ */
+virReportSystemError(ENOSYS, %s,
+ _(Executing new processes is not supported on Win32
platform));
+return -1;
+}
+#endif
/*
* Run the command and wait for completion.
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Cast args/env for execve in virCommand
On Thu, May 12, 2011 at 10:37:04AM -0600, Eric Blake wrote: On 05/12/2011 10:29 AM, Daniel P. Berrange wrote: 'char **' is not compatible with 'const char* const*' so needs an explicit cast. Fixes the build on MinGW * src/util/command.c: Cast args/env for execve --- src/util/command.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 78586e8..d63b984 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1000,7 +1000,9 @@ int virCommandExec(virCommandPtr cmd) return -1; } -return execve(cmd-args[0], cmd-args, cmd-env); +return execve(cmd-args[0], + (const char *const *)cmd-args, + (const char *const *)cmd-env); ACK - this fixes things for now. But we really should be fixing the broken signature of mingw execve via a gnulib module (not yet written). Actually it doesn't work, because it breaks the native build instead. I've sent a different patch which just disables this, since it is unused on Win32 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/16] Introduce migration cookies to QEMU driver
On Mon, May 16, 2011 at 02:05:13PM +0100, Daniel P. Berrange wrote:
On Thu, May 12, 2011 at 01:17:21PM -0600, Eric Blake wrote:
On 05/12/2011 10:04 AM, Daniel P. Berrange wrote:
The migration protocol has support for a 'cookie' parameter which
is an opaque array of bytes as far as libvirt is concerned. Drivers
may use this for passing around arbitrary extra data they might
need during migration. The QEMU driver needs to do a few things:
+static int
+qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
+xmlXPathContextPtr ctxt,
+int flags ATTRIBUTE_UNUSED)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+char *tmp;
+
+/* We don't store the uuid, name, hostname, or hostuuid
+ * values. We just compare them to local data to do some
+ * sanity checking on migration operation
+ */
Should we do sanity checking that no unknown XML elements appear in the
cookie? And/or validate that flags contains no unexpected bits? That
is, should you insert virCheckFlags(0, -1) here, or in
qemuMigrationEatCookie? And rather than just using virXPathString to
probe whether a particular XML element is present, shouldn't you instead
do an iteration over all XML elements in order to detect unrecognized
elements?
Otherwise, what I'm afraid of is that the cookie eater (whether the
destination eating the cookie from Begin, or the source eating the
cookie from Prepare) will be running an earlier libvirt version than the
baker; if the baker added a mandatory flag to the cookie, but the eater
is unaware to look for that element and silently ignores it, then we
risk silent botching of migration.
Do we have enough infrastructure in place for source and destination to
agree on what features are mandatory vs. optional in the cookie, to
allow for omission of optional elements that would make migration nicer
but aren't fatal if left out? That is, a baker can always try a flag,
then retry without the flag, but retries can get expensive if there were
an alternative to first do a capability query to determine the common
subset of flags to use in the first place.
Well, the migration cookies were intended to be optional data,
which if omitted, don't result in bad stuff.
eg, if the graphics relocation data doesn't exist, the end user
will simply need to manually reconnect.
I think we do need some stricter checking on the lock state
data though, to validate that the same lock manager is
present on both sides.
@@ -342,6 +612,15 @@ qemuMigrationPrepareTunnel(struct qemud_driver
*driver,
event = virDomainEventNewFromObj(vm,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_MIGRATED);
+
+if (qemuMigrationBakeCookie(mig, driver, vm, cookieout,
cookieoutlen, 0) 0) {
+/* We could tear down the whole guest here, but
+ * cookie data is (so far) non-critical, so that
+ * seems a little harsh. We'll just warn for now.
+ */
+VIR_WARN(Unable to encode migration cookie);
+}
+
ret = 0;
endjob:
@@ -369,7 +648,7 @@ cleanup:
virDomainObjUnlock(vm);
if (event)
qemuDomainEventQueue(driver, event);
-qemuDriverUnlock(driver);
+qemuMigrationCookieFree(mig);
Umm, did you really intend to drop the qemuDriverUnlock line?
No, that's a merge rebase error
Actually no it wasn't. This is a bug that's being fixed. The caller of
this method, already unlocks the driver on completion, so we had a
double unlock.
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/16] Introduce migration cookies to QEMU driver
On 05/16/2011 08:00 AM, Daniel P. Berrange wrote: @@ -369,7 +648,7 @@ cleanup: virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); -qemuDriverUnlock(driver); +qemuMigrationCookieFree(mig); Umm, did you really intend to drop the qemuDriverUnlock line? No, that's a merge rebase error Actually no it wasn't. This is a bug that's being fixed. The caller of this method, already unlocks the driver on completion, so we had a double unlock. Then let's split that into a separate patch; bug fixes and new features in the same patch are harder to deal with during bisection. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Disable virCommandExec on Win32
On 05/16/2011 07:22 AM, Daniel P. Berrange wrote: Mingw execve() has a broken signature. Disable this function until gnulib fixes the signature, since we don't really need this on Win32 anyway. * src/util/command.c: Disable virCommandExec on Win32 --- src/util/command.c | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] KVM Forum 2011: Call For Participation
The abstract submission deadline was originally set for today. The forum committee agreed to extend the deadline period until next Sunday, May 22. The notification date remains the same (May 31). Thanks, your KVM Forum 2011 Program Committee On 04/21/2011 08:21 PM, kvm-forum-2011...@redhat.com wrote: = KVM Forum 2011: Call For Participation August 15-16, 2011 - Hyatt Regency Vancouver - Vancouver, Canada = KVM is an industry leading open source hypervisor that provides an ideal platform for datacenter virtualization, virtual desktop infrastructure, and cloud computing. Once again, it's time to bring together the community of developers and users that define the KVM ecosystem for our annual technical conference. We will discuss the current state of affairs and plan for the future of KVM, its surrounding infrastructure, and management tools. So mark your calendar and join us in advancing KVM. http://events.linuxfoundation.org/events/kvm-forum/ We are colocated with The Linux Foundation's LinuxCon again this year. KVM Forum attendees will be eligible to attend LinuxCon for a discounted rate. http://events.linuxfoundation.org/events/kvm-forum/register We invite you to lead part of the discussion by submitting a speaking proposal for KVM Forum 2011. http://events.linuxfoundation.org/cfp Suggested topics: KVM - Scaling and performance - Nested virtualization - I/O improvements - PCI device assignment - Driver domains - Time keeping - Resource management (cpu, memory, i/o) - Memory management (page sharing, swapping, huge pages, etc) - Fault tolerance - VEPA, VN-Link, vswitch - Security QEMU - Device model improvements - New devices - Scaling and performance - Desktop virtualization - Spice - Increasing robustness and hardening - Security model - Management interfaces - QMP protocol and implementation - Image formats - Live migration - Live snapshots and merging Virtio - Speeding up existing devices - Alternatives - Virtio on non-Linux Management infrastructure - Libvirt - KVM autotest - OpenStack - Network virtualization management - Enterprise storage management Cloud computing - Scalable storage - Virtual networking - Security - Provisioning - Hybrid SUBMISSION REQUIREMENTS Abstracts due: May 16th, 2011 Notification: May 31th, 2011 Please submit a short abstract (~150 words) describing your presentation proposal. In your submission please note how long your talk will take. Slots vary in length up to 45 minutes. Also include in your proposal the proposal type -- one of: - technical talk - end-user talk - BOF (birds of a feather) session Sumbit your proposal here: http://events.linuxfoundation.org/cfp You will receive a notification whether or not your presentation proposal was accepted by May 31st. END-USER COLLABORATION One of the big challenges as developers is to know what, where and how people actually use our software. We will reserve a few slots for end users talking about their deployment challenges and achievements. If you are using KVM in production you are encouraged submit a speaking proposal. Simply mark it as an end-user collaboration proposal. As and end user, this is a unique opportunity to get your input to developers. BOF SESSION We will reserve some slots in the evening after the main conference tracks, for birds of a feather sessions. These sessions will be less formal than presentation tracks and targetted for people who would like to discuss specific issues with other developers and/or users. If you are interested in getting developers and/or uses together to discuss a specific problem, please submit a BOF proposal. LIGHTNING TALKS In addition to submitted talks we will also have some room for lightning talks. These are short (5 minute) discussions to highlight new work or ideas that aren't complete enough to warrant a full presentation slot. Lightning talk submissions and scheduling will be handled on-site at KVM Forum. HOTEL / TRAVEL The KVM Forum 2011 will be held in Vancouver BC at the Hyatt Regency Vancouver. http://events.linuxfoundation.org/events/kvm-forum/travel Thank you for your interest in KVM. We're looking forward to your submissions and seeing you at the KVM Forum 2011 in August! Thanks, your KVM Forum 2011 Program Commitee Please contact us with any questions or comments. kvm-forum-2011...@redhat.com ___ Kvm-forum-2011-pc mailing list kvm-forum-2011...@redhat.com https://www.redhat.com/mailman/listinfo/kvm-forum-2011-pc -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/16 v5] Implement migration v3 protocol
On Thu, May 12, 2011 at 05:04:36PM +0100, Daniel P. Berrange wrote: An update of http://www.redhat.com/archives/libvir-list/2011-May/msg00605.html Changes in this series - Fix comments from previous review - Rebase to latest GIT Also pullable from http://gitorious.org/~berrange/libvirt/staging/commits/migrate-locking-3 NB, this branch includes the migration + lock manager series in one big set. I've pushed this series now. There will be a couple of follow up patches to add some further enhancements before next release. 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] send 'container=libvirt' in env to container init
This allows upstart scripts to detect that they are in a container and modify their behavior accordingly. In this way, the same ubuntu maverick or natty image with the 'lxcguest' package installed can be booted on bare metal, in kvm, or as a libvirt or liblxc container. Signed-off-by: Chuck Short zul...@ubuntu.com Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com --- src/lxc/lxc_container.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9ae93b5..a70aeeb 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -112,6 +112,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=libvirt); virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); -- 1.7.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] send 'container=libvirt' in env to container init
On 05/16/2011 08:47 AM, Serge Hallyn wrote: This allows upstart scripts to detect that they are in a container and modify their behavior accordingly. In this way, the same ubuntu maverick or natty image with the 'lxcguest' package installed can be booted on bare metal, in kvm, or as a libvirt or liblxc container. Signed-off-by: Chuck Short zul...@ubuntu.com Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com --- src/lxc/lxc_container.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9ae93b5..a70aeeb 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -112,6 +112,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=libvirt); POSIX reserves lowercase env names for the user. Is upstart really using a lower case name, or should this be an upper case name? virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); Can upstart be taught to look for LIBIVRT_LXC_UUID instead? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] virsh: Correctly initialize libvirt
virsh didn't call virInitialize(), which (among other things)
initializes virLastErr thread local variable. As a result of that, virsh
could just segfault in virEventRegisterDefaultImpl() since that is the
first call that touches (resets) virLastErr.
I have no idea what lucky coincidence made this bug visible but I was
able to reproduce it in 100% cases but only in one specific environment
which included building in sandbox.
---
src/libvirt.c |3 +++
tools/virsh.c | 13 +
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 5a5439d..787908e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -369,6 +369,9 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
* in multithreaded applications to avoid potential race when initializing
* the library.
*
+ * Calling virInitialize is mandatory, unless your first API call is one of
+ * virConnectOpen*.
+ *
* Returns 0 in case of success, -1 in case of error
*/
int
diff --git a/tools/virsh.c b/tools/virsh.c
index e35637d..b469e7a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12983,6 +12983,10 @@ main(int argc, char **argv)
char *defaultConn;
bool ret = true;
+memset(ctl, 0, sizeof(vshControl));
+ctl-imode = true; /* default is interactive mode */
+ctl-log_fd = -1; /* Initialize log file descriptor */
+
if (!setlocale(LC_ALL, )) {
perror(setlocale);
/* failure to setup locale is not fatal */
@@ -12996,15 +13000,16 @@ main(int argc, char **argv)
return EXIT_FAILURE;
}
+if (virInitialize() 0) {
+vshError(ctl, %s, _(Failed to initialize libvirt));
+return EXIT_FAILURE;
+}
+
if (!(progname = strrchr(argv[0], '/')))
progname = argv[0];
else
progname++;
-memset(ctl, 0, sizeof(vshControl));
-ctl-imode = true; /* default is interactive mode */
-ctl-log_fd = -1; /* Initialize log file descriptor */
-
if ((defaultConn = getenv(VIRSH_DEFAULT_CONNECT_URI))) {
ctl-name = vshStrdup(ctl, defaultConn);
}
--
1.7.5.rc3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] virsh: Correctly initialize libvirt
On 05/16/2011 08:59 AM, Jiri Denemark wrote: virsh didn't call virInitialize(), which (among other things) initializes virLastErr thread local variable. As a result of that, virsh could just segfault in virEventRegisterDefaultImpl() since that is the first call that touches (resets) virLastErr. I have no idea what lucky coincidence made this bug visible but I was able to reproduce it in 100% cases but only in one specific environment which included building in sandbox. --- src/libvirt.c |3 +++ tools/virsh.c | 13 + 2 files changed, 12 insertions(+), 4 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [virt-devel] Cache and memory bandwidth graphs: native versus guest
Bill Gray wrote: See attachment with two graphs: (1) cache bandwidth, (2) blowup of sustained memory bandwidth region... Bill, I had some difficulty with this document under ooffice. A recent version seized and an older version didn't seem to render correctly. Could you export it as pdf? Thanks, -john - X axis has a log scale - Light blue line is an older system with 32K L1 and 6M L2 caches - All other measurements on perf34: 32K L1, 256K L2, 30M L3 caches - Majority of variation in L1 cache region is from the two guest measurements done with no taskset to a VCPU: yellow and maroon lines. Perhaps this reflects the test bouncing between VCPUs in the guest. - The sustained memory bandwidth for the guest with no pinning is only 80% of native (maroon line), which motivates more convenient and comprehensive numactl for guests. - Virtualized bandwidth is otherwise nearly in line with native, which confirms the importance of the virtual CPUID communicating actual native cache sizes to cache-size-aware guest applications, since guest apps could benefit from the full size of the native cache. (Guest was started with -cpu host, but lscpu in guest showed 4M cache despite actual 30M cache.) -- john.coo...@redhat.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] send 'container=libvirt' in env to container init
Quoting Eric Blake (ebl...@redhat.com): On 05/16/2011 08:47 AM, Serge Hallyn wrote: This allows upstart scripts to detect that they are in a container and modify their behavior accordingly. In this way, the same ubuntu maverick or natty image with the 'lxcguest' package installed can be booted on bare metal, in kvm, or as a libvirt or liblxc container. Signed-off-by: Chuck Short zul...@ubuntu.com Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com --- src/lxc/lxc_container.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 9ae93b5..a70aeeb 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -112,6 +112,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=libvirt); POSIX reserves lowercase env names for the user. Is upstart really using a lower case name, or should this be an upper case name? No upstart isn't doing anything itself :) Just blame me - the upstart scripts I put into lxcguest do in fact check for lower-case. What do you mean by 'for the user'? For the user to type into the boot args, or something else? virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); Can upstart be taught to look for LIBIVRT_LXC_UUID instead? That's a possibility, yes. It just makes it less symmetric to the liblxc case, but if you prefer that I see no reason why it wouldn't work. thanks, -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
On Fri, May 13, 2011 at 05:05:14PM -0600, Eric Blake wrote: On 05/13/2011 04:11 AM, Federico Simoncelli wrote: Originally most of libvirt domain-specific calls were blocking during a migration. A new mechanism to allow specific calls (blkstat/blkinfo) to be executed in such condition has been implemented. In the long term it'd be desirable to get a more general solution to mark further APIs as migration safe, without needing special case code. * src/qemu/qemu_migration.c: add some additional job signal flags for doing blkstat/blkinfo during a migration * src/qemu/qemu_domain.c: add a condition variable that can be used to efficiently wait for the migration code to clear the signal flag * src/qemu/qemu_driver.c: execute blkstat/blkinfo using the job signal flags during migration --- src/qemu/qemu_domain.c| 13 ++ src/qemu/qemu_domain.h|9 src/qemu/qemu_driver.c| 103 - src/qemu/qemu_migration.c | 38 - 4 files changed, 131 insertions(+), 32 deletions(-) ACK; but pending danpb's commits actually going in. I've pushed my migration series, so this can be rebased now 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] maint: ignore generated file
* .gitignore: Ignore recently added file. --- I'm pushing this as a trivial followup. .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 87a105f..a15100c 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,7 @@ /configure /configure.lineno /daemon/*_dispatch_*.h +/docs/hvsupport.html.in /gnulib/ /libtool /libvirt-*.tar.gz -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] Automatically generate the hvsupport.html.in file from source files
On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is forever out of date. By annotating the driver struct tables in each driver with version information, we can auto-generate the hvsupport.html.in file. Annotating the drivers will be mandatory for new patches, ensuring hvsupport.html.in is never out of date again. * docs/hvsupport.html.in: Delete * hvsupport.pl: Script to generate hvsupport.html.in * Makefile.am: Autogenerate hvsupport.html.in --- docs/Makefile.am | 10 +- docs/hvsupport.html.in | 801 docs/hvsupport.pl | 353 + 3 files changed, 360 insertions(+), 804 deletions(-) delete mode 100644 docs/hvsupport.html.in create mode 100644 docs/hvsupport.pl Hmm, that should probably be mode 100755, since it is an executable script. I'm pushing that obvious fix. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] Automatically generate the hvsupport.html.in file from source files
On Mon, May 16, 2011 at 10:00:30AM -0600, Eric Blake wrote: On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is forever out of date. By annotating the driver struct tables in each driver with version information, we can auto-generate the hvsupport.html.in file. Annotating the drivers will be mandatory for new patches, ensuring hvsupport.html.in is never out of date again. * docs/hvsupport.html.in: Delete * hvsupport.pl: Script to generate hvsupport.html.in * Makefile.am: Autogenerate hvsupport.html.in --- docs/Makefile.am | 10 +- docs/hvsupport.html.in | 801 docs/hvsupport.pl | 353 + 3 files changed, 360 insertions(+), 804 deletions(-) delete mode 100644 docs/hvsupport.html.in create mode 100644 docs/hvsupport.pl Hmm, that should probably be mode 100755, since it is an executable script. I'm pushing that obvious fix. It doesn't hugely matter, since the makefile.am explicitly calls '$(PERL) hvsupport.pl' 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] maint: mark more perl scripts executable
* src/remote/rpcgen_fix.pl: Add executable bit. * tests/oomtrace.pl: Likewise. --- I found a couple other files that could also use this treatment. Pushing under the trivial rule. 0 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 = 100755 src/remote/rpcgen_fix.pl mode change 100644 = 100755 tests/oomtrace.pl diff --git a/src/remote/rpcgen_fix.pl b/src/remote/rpcgen_fix.pl old mode 100644 new mode 100755 diff --git a/tests/oomtrace.pl b/tests/oomtrace.pl old mode 100644 new mode 100755 -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] Automatically generate the hvsupport.html.in file from source files
On 05/16/2011 10:11 AM, Daniel P. Berrange wrote: On Mon, May 16, 2011 at 10:00:30AM -0600, Eric Blake wrote: On 05/13/2011 07:36 AM, Daniel P. Berrange wrote: The hvsupport.html.in file is forever out of date. By annotating the driver struct tables in each driver with version information, we can auto-generate the hvsupport.html.in file. Annotating the drivers will be mandatory for new patches, ensuring hvsupport.html.in is never out of date again. * docs/hvsupport.html.in: Delete * hvsupport.pl: Script to generate hvsupport.html.in * Makefile.am: Autogenerate hvsupport.html.in --- docs/Makefile.am | 10 +- docs/hvsupport.html.in | 801 docs/hvsupport.pl | 353 + 3 files changed, 360 insertions(+), 804 deletions(-) delete mode 100644 docs/hvsupport.html.in create mode 100644 docs/hvsupport.pl Hmm, that should probably be mode 100755, since it is an executable script. I'm pushing that obvious fix. It doesn't hugely matter, since the makefile.am explicitly calls '$(PERL) hvsupport.pl' True, but we might as well be consistent: -rwxrwxr-x. 1 dummy dummy 45492 May 12 09:56 ./daemon/remote_generator.pl* -rw-rw-r--. 1 dummy dummy 9521 May 16 09:23 ./docs/hvsupport.pl -rwxrwxr-x. 1 dummy dummy 3290 Mar 14 14:36 ./docs/todo.pl* -rw-rw-r--. 1 dummy dummy 3083 May 9 10:14 ./src/remote/rpcgen_fix.pl -rw-rw-r--. 1 dummy dummy 688 Nov 1 2010 ./tests/oomtrace.pl Before-hand, 2 out of 5 had the bit set; yet tests/oomtrace.pl is _not_ invoked via $(PERL) in tests/Makefile.am. So now all 5 scripts have the bit set. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt, FreeBSD portability inquiry
On Sun, May 15, 2011 at 12:50:30PM -0700, Jason Helfman thus spake: 2011/5/14 Jason Helfman jhelf...@e-e.com: Compiles fine now. Matthias does a make install work, and does it function as a library? Thanks! Yes works. I can use virsh and connect to libvirtd on another box. The only problem I currently have is that virsh fails to enter interactive mode because sysconf(_SC_GETPW_R_SIZE_MAX) fails, but we'll get that fixed. Matthias Great news. Would you mind submitting a followup to the problem report I opened at FreeBSD regarding this? There is a link at the bottom of the report. http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/157024 Thanks, Jason This PR has been picked up by someone @FreeBSD, and is being worked on. Any follow-up traffic to this url would be appreciated in documenting that it works, and the capabilities of it at the moment. Thanks! -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: allow blkstat/blkinfo calls during migration
On 05/13/2011 04:11 AM, Federico Simoncelli wrote:
Originally most of libvirt domain-specific calls were blocking
during a migration.
A new mechanism to allow specific calls (blkstat/blkinfo) to be
executed in such condition has been implemented.
In the long term it'd be desirable to get a more general
solution to mark further APIs as migration safe, without needing
special case code.
Actually, we now need a v4, although I'll try to post one shortly.
@@ -123,6 +135,7 @@ static void qemuDomainObjPrivateFree(void *data)
virDomainChrSourceDefFree(priv-monConfig);
VIR_FREE(priv-vcpupids);
VIR_FREE(priv-lockState);
+ignore_value(virCondDestroy(priv-signalCond));
We should also add a virCondDestroy for priv-jobCond.
+if ((priv-jobActive == QEMU_JOB_MIGRATION_OUT)
+|| (priv-jobActive == QEMU_JOB_SAVE)) {
+virDomainObjRef(vm);
+while (priv-jobSignals QEMU_JOB_SIGNAL_BLKSTAT)
+ignore_value(virCondWait(priv-signalCond, vm-lock));
+
+priv-jobSignals |= QEMU_JOB_SIGNAL_BLKSTAT;
+priv-jobSignalsData.statDevName = disk-info.alias;
+priv-jobSignalsData.blockStat = stats;
+priv-jobSignalsData.statRetCode = -1;
+
For safety sake, I'd rather see the jobSignals assignment after the
jobSignalsData assignments. I'm not sure if we need some sort of memory
barrier to ensure that a compiler (and/or hardware) won't rearrange the
assignments to complete out of order, but we really don't want the
migration thread to see the jobSignalsData contents until after they are
stable.
@@ -796,8 +824,12 @@ qemuMigrationWaitForCompletion(struct qemud_driver
*driver, virDomainObjPtr vm)
job = _(job);
}
-if (qemuMigrationProcessJobSignals(driver, vm, job) 0)
-goto cleanup;
+while (priv-jobSignals) {
+if (qemuMigrationProcessJobSignals(driver, vm, job) 0)
+goto cleanup;
+}
This while loop continues until priv-jobSignals is 0 _or_ an error
occurs...
+
+virCondSignal(priv-signalCond);
if (qemuMigrationUpdateJobStatus(driver, vm, job) 0)
goto cleanup;
@@ -813,6 +845,8 @@ qemuMigrationWaitForCompletion(struct qemud_driver
*driver, virDomainObjPtr vm)
}
cleanup:
+virCondBroadcast(priv-signalCond);
+
...therefore, on error, this can raise the condition variable while
jobSignals is non-zero. We've just introduced a deadlock:
thread 1 starts a migration
thread 2 queries block info, sets the jobSignals bit, and waits for the
cond variable and the bit to be clear
something goes wrong with migration (suppose qemu is killed externally)
thread 1 finally gets to qemuMigrationUpdateJobStatus, but sees that vm
is no longer live so it exits with error
thread 2 is now stuck waiting for the jobSignals to clear, but thread 1
is no longer going to clear it
I'm still working on the right way to fix it, but I think that
qemuMigrationProcessJobSignals needs a bool parameter that says whether
it is in cleanup mode, in which case it always clears at least one
jobSignals bit even on error, and for blkinfo/blkstat, it sets the ret
value to -1 before clearing the bit. That way,
qemuMigrationWaitForCompletion always ends with jobSignals == 0 and
driver lock held, and as long as the driver lock is then not released
until jobActive has been reset, then no new qemudDomainBlockStats will
start, and the existing one in thread 2 will correctly fail for the same
reason that migration in thread 1 failed (that is, the vm went away early).
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] screenshot: Expose the new API in virsh
* tools/virsh.c: Add screenshot command
* tools/virsh.pod: Document new command
* src/libvirt.c: Fix off-be-one error
---
diff to v1:
- make filename optional and generate filename when missing
src/libvirt.c |2 +-
tools/virsh.c | 152 +++
tools/virsh.pod |9 +++
3 files changed, 162 insertions(+), 1 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 5a5439d..cfb9e3b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2461,7 +2461,7 @@ error:
* The screen ID is the sequential number of screen. In case of multiple
* graphics cards, heads are enumerated before devices, e.g. having
* two graphics cards, both with four heads, screen ID 5 addresses
- * the first head on the second card.
+ * the second head on the second card.
*
* Returns a string representing the mime-type of the image format, or
* NULL upon error. The caller must free() the returned value.
diff --git a/tools/virsh.c b/tools/virsh.c
index e35637d..aad930e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1879,6 +1879,157 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+static const vshCmdInfo info_screenshot[] = {
+{help, N_(take a screenshot of a current domain console and store it
+into a file)},
+{desc, N_(screenshot of a current domain console)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_screenshot[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{file, VSH_OT_DATA, VSH_OFLAG_NONE, N_(where to store the screenshot)},
+{screen, VSH_OT_INT, VSH_OFLAG_NONE, N_(ID of a screen to take
screenshot of)},
+{NULL, 0, 0, NULL}
+};
+
+static int cmdScreenshotSink(virStreamPtr st ATTRIBUTE_UNUSED,
+ const char *bytes, size_t nbytes, void *opaque)
+{
+int *fd = opaque;
+
+return safewrite(*fd, bytes, nbytes);
+}
+
+/**
+ * Generate string: 'domain name-timestamp[extension]'
+ */
+static char *
+vshGenerFileName(vshControl *ctl, virDomainPtr dom) {
+char timestr[100];
+struct timeval cur_time;
+struct tm time_info;
+const char *ext = NULL;
+const char *hypType = NULL;
+char *ret = NULL;
+
+/* We should be already connected, but doesn't
+ * hurt to check */
+if (!vshConnectionUsability(ctl, ctl-conn))
+return NULL;
+
+if (!dom) {
+vshError(ctl, %s, _(Invalid domain supplied));
+return NULL;
+}
+
+if (!(hypType = virConnectGetType(ctl-conn))) {
+vshError(ctl, %s, _(Can't query hypervisor's type));
+return NULL;
+}
+
+if (STREQ(hypType, QEMU))
+ext = .ppm;
+else if (STREQ(hypType, VBOX))
+ext = .png;
+/* add hypervisors here */
+
+gettimeofday(cur_time, NULL);
+localtime_r(cur_time.tv_sec, time_info);
+strftime(timestr, sizeof(timestr), %Y-%m-%d-%H:%M:%S, time_info);
+
+if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom),
+timestr, ext ? ext : ) 0) {
+vshError(ctl, %s, _(Out of memory));
+return false;
+}
+
+return ret;
+}
+
+static bool
+cmdScreenshot(vshControl *ctl, const vshCmd *cmd) {
+virDomainPtr dom;
+const char *name = NULL;
+char *file = NULL;
+int fd = -1;
+virStreamPtr st = NULL;
+unsigned int screen = 0;
+unsigned int flags = 0; /* currently unused */
+int ret = false;
+bool created = true;
+bool generated = false;
+char *mime = NULL;
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return false;
+
+if (vshCommandOptString(cmd, file, (const char **) file) 0) {
+vshError(ctl, %s, _(file must not be empty));
+return false;
+}
+
+if (vshCommandOptInt(cmd, screen, (int*) screen) 0) {
+vshError(ctl, %s, _(invalid screen ID));
+return false;
+}
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, name)))
+return false;
+
+if (!file) {
+if (!(file=vshGenerFileName(ctl, dom)))
+return false;
+generated = true;
+}
+
+if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) 0) {
+created = false;
+if (errno != EEXIST ||
+(fd = open(file, O_WRONLY|O_TRUNC, 0666)) 0) {
+vshError(ctl, _(cannot create file %s), file);
+goto cleanup;
+}
+}
+
+st = virStreamNew(ctl-conn, 0);
+
+mime = virDomainScreenshot(dom, st, screen, flags);
+if (mime == NULL) {
+vshError(ctl, _(could not take a screenshot of %s), name);
+goto cleanup;
+}
+
+if (virStreamRecvAll(st, cmdScreenshotSink, fd) 0) {
+vshError(ctl, _(could not receive data from domain %s), name);
+goto cleanup;
+}
+
+if (VIR_CLOSE(fd) 0) {
+vshError(ctl, _(cannot close file %s), file);
+goto cleanup;
+}
+
+if (virStreamFinish(st) 0) {
+vshError(ctl, _(cannot close stream on domain %s), name);
+goto
[libvirt] [PATCH] Fix remote dispatcher for screenshot command
* daemon/remote.c: Update screenshot dispatcher to follow
standard practice
---
daemon/remote.c | 53 +
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 181f149..bb01c89 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1694,59 +1694,64 @@ no_memory:
}
static int
-remoteDispatchDomainScreenshot (struct qemud_server *server ATTRIBUTE_UNUSED,
-struct qemud_client *client,
-virConnectPtr conn,
-remote_message_header *hdr,
-remote_error *rerr,
-remote_domain_screenshot_args *args,
-remote_domain_screenshot_ret *ret)
+remoteDispatchDomainScreenshot(struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ virConnectPtr conn,
+ remote_message_header *hdr,
+ remote_error *rerr,
+ remote_domain_screenshot_args *args,
+ remote_domain_screenshot_ret *ret)
{
int rv = -1;
struct qemud_client_stream *stream = NULL;
-virDomainPtr dom;
+virDomainPtr dom = NULL;
char *mime, **mime_p;
+if (!conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
ret-mime = NULL;
-dom = get_nonnull_domain (conn, args-dom);
-if (dom == NULL)
-goto err;
+if (!(dom = get_nonnull_domain (conn, args-dom)))
+goto cleanup;
-stream = remoteCreateClientStream(conn, hdr);
-if (!stream)
-goto err;
+if (!(stream = remoteCreateClientStream(conn, hdr)))
+goto cleanup;
-mime = virDomainScreenshot(dom, stream-st, args-screen, args-flags);
-if (!mime)
-goto err;
+if (!(mime = virDomainScreenshot(dom, stream-st, args-screen,
args-flags)))
+goto cleanup;
if (remoteAddClientStream(client, stream, 1) 0) {
virStreamAbort(stream-st);
-goto err;
+goto cleanup;
}
if (VIR_ALLOC(mime_p) 0) {
-remoteDispatchOOMError(rerr);
+virReportOOMError();
goto cleanup;
}
*mime_p = strdup(mime);
if (*mime_p == NULL) {
-remoteDispatchOOMError(rerr);
+virReportOOMError();
goto cleanup;
}
ret-mime = mime_p;
+
rv = 0;
-err:
+cleanup:
if (rv 0)
remoteDispatchError(rerr);
-cleanup:
-virDomainFree(dom);
-if (stream rv != 0)
+if (dom)
+virDomainFree(dom);
+if (stream rv != 0) {
+virStreamAbort(stream-st);
remoteFreeClientStream(client, stream);
+}
return rv;
}
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Report an error when virGetUserDirectory fails
2011/5/16 Laine Stump la...@laine.org:
On 05/15/2011 01:31 AM, Matthias Bolte wrote:
Otherwise virsh shows the interactive greeting and the silently exists
instead of enterting interactive mode.
---
tools/virsh.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3baa015..356e0ae 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12495,8 +12495,10 @@ vshReadlineInit(vshControl *ctl)
/* Prepare to read/write history from/to the ~/.virsh/history file */
userdir = virGetUserDirectory(getuid());
- if (userdir == NULL)
+ if (userdir == NULL) {
+ vshError(ctl, %s, _(Could not determine home directory));
return -1;
+ }
if (virAsprintf(ctl-historydir, %s/.virsh, userdir) 0) {
vshError(ctl, %s, _(Out of memory));
ACK.
Thanks, I fixed two typos in the commit message and pushed the result.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cleanup: use c99 initializations
On Mon, May 16, 2011 at 04:17:54PM +0800, Lai Jiangshan wrote: It help us avoid to touch many files when we define the internal APIs of a public API like commit b19bd85e15332405623ed46a3ee8b99812aa6007. --- src/esx/esx_device_monitor.c | 15 +-- src/esx/esx_driver.c | 175 +++--- src/esx/esx_interface_driver.c | 18 +--- src/esx/esx_network_driver.c | 23 + src/esx/esx_nwfilter_driver.c | 13 +-- src/esx/esx_secret_driver.c| 15 +-- src/esx/esx_storage_driver.c | 68 +--- src/interface/netcf_driver.c | 30 +++--- src/libxl/libxl_driver.c | 165 +--- src/lxc/lxc_driver.c | 161 --- src/network/bridge_driver.c| 40 src/openvz/openvz_driver.c | 152 +++--- src/phyp/phyp_driver.c | 136 +-- src/qemu/qemu_driver.c | 220 ++--- src/remote/remote_driver.c | 224 +++--- src/test/test_driver.c | 240 src/uml/uml_driver.c | 156 --- src/vbox/vbox_tmpl.c | 208 +++ src/vmware/vmware_driver.c | 140 +--- src/xen/xen_driver.c | 188 +--- src/xen/xen_hypervisor.c | 56 +++--- src/xen/xen_inotify.c | 40 +--- src/xen/xend_internal.c| 73 ++--- src/xen/xm_internal.c | 52 +++--- src/xen/xs_internal.c | 47 ++--- src/xenapi/xenapi_driver.c | 157 --- 26 files changed, 958 insertions(+), 1854 deletions(-) The same change was already posted last week and is now merged commit 879d409e9e618ef2bbe92a8c4ff08017f5bc1793 Author: Daniel P. Berrange berra...@redhat.com Date: Fri May 13 11:16:31 2011 +0100 Convert all driver struct intializers to C99 style commit 9b1ae97fdc230facf58ea4a290305c038e32b147 Author: Daniel P. Berrange berra...@redhat.com Date: Fri May 13 14:35:01 2011 +0100 Add many version number annotations to drivers 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] [PATCHv4 2/6] virNodeGetCPUTimeParameters: Define internal driver API
On Mon, May 16, 2011 at 11:36:14AM +0900, Minoru Usui wrote: virNodeGetCPUTimeParameters: Define internal driver API Signed-off-by: Minoru Usui u...@mxm.nes.nec.co.jp --- src/driver.h |8 src/esx/esx_driver.c |1 + src/libxl/libxl_driver.c |1 + src/lxc/lxc_driver.c |1 + src/openvz/openvz_driver.c |1 + src/phyp/phyp_driver.c |1 + src/qemu/qemu_driver.c |1 + src/remote/remote_driver.c |1 + src/test/test_driver.c |1 + src/uml/uml_driver.c |1 + src/vbox/vbox_tmpl.c |1 + src/vmware/vmware_driver.c |1 + src/xen/xen_driver.c |1 + src/xenapi/xenapi_driver.c |1 + 14 files changed, 21 insertions(+), 0 deletions(-) Unfortunately, this patch will need updating again to rebase to last GIT. Essentially you can remove all the changes in this patch, except for the one against src/driver.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] [PATCH] Fix remote dispatcher for screenshot command
On 05/16/2011 11:12 AM, Daniel P. Berrange wrote:
* daemon/remote.c: Update screenshot dispatcher to follow
standard practice
---
daemon/remote.c | 53 +
1 files changed, 29 insertions(+), 24 deletions(-)
ACK.
if (VIR_ALLOC(mime_p) 0) {
-remoteDispatchOOMError(rerr);
Meanwhile, we really should nuke this function, now that no one uses it.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 4/6] virNodeGetCPUTimeParameters: Implement remote protocol
On Mon, May 16, 2011 at 11:37:34AM +0900, Minoru Usui wrote:
virNodeGetCPUTimeParameters: Implement remote protocol
Signed-off-by: Minoru Usui u...@mxm.nes.nec.co.jp
---
daemon/remote.c | 81
++
src/remote/remote_driver.c | 64 +
src/remote/remote_protocol.x | 21 ++-
3 files changed, 165 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 6e13958..f7b84d9 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1749,6 +1749,87 @@ cleanup:
return rv;
}
+static int
+remoteDispatchNodeGetCPUTimeParameters (struct qemud_server *server
+ATTRIBUTE_UNUSED,
+struct qemud_client *client
+ATTRIBUTE_UNUSED,
+virConnectPtr conn,
+remote_message_header *
+hdr ATTRIBUTE_UNUSED,
+remote_error *rerr,
+
remote_node_get_cpu_time_parameters_args *
+args,
+
remote_node_get_cpu_time_parameters_ret *
+ret)
This is horribly whitespace mangled. Please put the variable names + annotations
on the same line as the corresponding data types.
+{
+virCPUTimeParameterPtr params = NULL;
+int i;
+int nparams = args-nparams;
+unsigned int flags;
+int rv = -1;
+
+if (!conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+flags = args-flags;
+
+if (nparams REMOTE_NODE_CPU_TIME_PARAMETERS_MAX) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large));
+goto cleanup;
+}
+if (VIR_ALLOC_N(params, nparams) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (virNodeGetCPUTimeParameters(conn, params, nparams, flags) 0)
+goto cleanup;
+
+/* In this case, we need to send back the number of parameters
+ * supported
+ */
+if (args-nparams == 0) {
+ret-nparams = nparams;
+goto success;
+}
+
+/* Serialise the memory parameters. */
+ret-params.params_len = nparams;
+if (VIR_ALLOC_N(ret-params.params_val, nparams) 0)
+goto no_memory;
+
+for (i = 0; i nparams; ++i) {
+/* remoteDispatchClientRequest will free this: */
+ret-params.params_val[i].field = strdup(params[i].field);
+if (ret-params.params_val[i].field == NULL)
+goto no_memory;
+
+ret-params.params_val[i].value = params[i].value;
+}
+
+success:
+rv = 0;
+
+cleanup:
+if (rv 0) {
+remoteDispatchError(rerr);
+if (ret-params.params_val) {
+for (i = 0; i nparams; i++)
+VIR_FREE(ret-params.params_val[i].field);
+VIR_FREE(ret-params.params_val);
+}
+}
+VIR_FREE(params);
+return rv;
+
+no_memory:
+virReportOOMError();
+goto cleanup;
+}
+
/*-*/
static int
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index f61afdc..46130ac 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1724,6 +1724,70 @@ done:
}
static int
+remoteNodeGetCPUTimeParameters (virConnectPtr conn,
+virCPUTimeParameterPtr params, int *nparams,
+unsigned int flags)
+{
+int rv = -1;
+remote_node_get_cpu_time_parameters_args args;
+remote_node_get_cpu_time_parameters_ret ret;
+int i = -1;
+struct private_data *priv = conn-privateData;
+
+remoteDriverLock(priv);
+
+args.nparams = *nparams;
+args.flags = flags;
+
+memset (ret, 0, sizeof ret);
+if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_TIME_PARAMETERS,
+ (xdrproc_t) xdr_remote_node_get_cpu_time_parameters_args,
+ (char *) args,
+ (xdrproc_t) xdr_remote_node_get_cpu_time_parameters_ret,
+ (char *) ret) == -1)
+goto done;
+
+/* Check the length of the returned list carefully. */
+if (ret.params.params_len REMOTE_NODE_CPU_TIME_PARAMETERS_MAX ||
+ret.params.params_len *nparams) {
+remoteError(VIR_ERR_RPC, %s,
+_(remoteNodeGetCPUTimeParameters:
+ returned number of parameters exceeds limit));
+goto cleanup;
+}
+/* Handle the case when the caller does not know the number of parameters
+ * and is asking for the number of parameters supported
+ */
+if
[libvirt] [PATCH] Fix leak of mime type string in screenshot dispatcher
* daemon/remote.c: Free mime string
---
daemon/remote.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index bb01c89..ea36bf5 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1746,6 +1746,7 @@ remoteDispatchDomainScreenshot(struct qemud_server
*server ATTRIBUTE_UNUSED,
cleanup:
if (rv 0)
remoteDispatchError(rerr);
+VIR_FREE(mime);
if (dom)
virDomainFree(dom);
if (stream rv != 0) {
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4] qemu: allow blkstat/blkinfo calls during migration
From: Federico Simoncelli fsimo...@redhat.com
Originally most of libvirt domain-specific calls were blocking
during a migration.
A new mechanism to allow specific calls (blkstat/blkinfo) to be
executed in such condition has been implemented.
In the long term it'd be desirable to get a more general
solution to mark further APIs as migration safe, without needing
special case code.
* src/qemu/qemu_migration.c: add some additional job signal
flags for doing blkstat/blkinfo during a migration
* src/qemu/qemu_domain.c: add a condition variable that can be
used to efficiently wait for the migration code to clear the
signal flag
* src/qemu/qemu_driver.c: execute blkstat/blkinfo using the
job signal flags during migration
---
My changes in v4:
Fix the mailmap (elided from this mail) for Federico's preferred address.
Add cleanup for jobCond.
Defer changes to jobSignals until after jobSignalData is stable.
Alter qemuMigrationProcessJobSignals to have a mode of operation
that guarantees that a jobSignals bit is cleared, regardless of
other errors.
.mailmap |1 +
AUTHORS |2 +-
src/qemu/qemu_domain.c| 14 ++
src/qemu/qemu_domain.h|9
src/qemu/qemu_driver.c| 107 +++-
src/qemu/qemu_migration.c | 76 +---
6 files changed, 159 insertions(+), 50 deletions(-)
diff --git a/.mailmap b/.mailmap
index f73e26b..2b98322 100644
--- a/.mailmap
+++ b/.mailmap
diff --git a/AUTHORS b/AUTHORS
index 1bb1f0f..47860a9 100644
--- a/AUTHORS
+++ b/AUTHORS
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bcacb18..8f4915c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -89,7 +89,19 @@ static void *qemuDomainObjPrivateAlloc(void)
if (VIR_ALLOC(priv) 0)
return NULL;
+if (virCondInit(priv-jobCond) 0)
+goto initfail;
+
+if (virCondInit(priv-signalCond) 0) {
+ignore_value(virCondDestroy(priv-jobCond));
+goto initfail;
+}
+
return priv;
+
+initfail:
+VIR_FREE(priv);
+return NULL;
}
static void qemuDomainObjPrivateFree(void *data)
@@ -101,6 +113,8 @@ static void qemuDomainObjPrivateFree(void *data)
qemuDomainPCIAddressSetFree(priv-pciaddrs);
virDomainChrSourceDefFree(priv-monConfig);
VIR_FREE(priv-vcpupids);
+ignore_value(virCondDestroy(priv-jobCond));
+ignore_value(virCondDestroy(priv-signalCond));
/* This should never be non-NULL if we get here, but just in case... */
if (priv-mon) {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6d24f53..af513e7 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -47,11 +47,19 @@ enum qemuDomainJobSignals {
QEMU_JOB_SIGNAL_SUSPEND = 1 1, /* Request VM suspend to finish live
migration offline */
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 2, /* Request migration downtime
change */
QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 3, /* Request migration speed change
*/
+QEMU_JOB_SIGNAL_BLKSTAT = 1 4, /* Request blkstat during migration */
+QEMU_JOB_SIGNAL_BLKINFO = 1 5, /* Request blkinfo during migration */
};
struct qemuDomainJobSignalsData {
unsigned long long migrateDowntime; /* Data for
QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME */
unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED
*/
+char *statDevName; /* Device name used by blkstat calls */
+virDomainBlockStatsPtr blockStat; /* Block statistics for
QEMU_JOB_SIGNAL_BLKSTAT */
+int statRetCode; /* Return code for the blkstat calls */
+char *infoDevName; /* Device name used by blkinfo calls */
+virDomainBlockInfoPtr blockInfo; /* Block information for
QEMU_JOB_SIGNAL_BLKINFO */
+int infoRetCode; /* Return code for the blkinfo calls */
};
typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
@@ -61,6 +69,7 @@ typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */
+virCond signalCond; /* Use to coordinate the safe queries during migration
*/
enum qemuDomainJob jobActive; /* Currently running job */
unsigned int jobSignals;/* Signals for running job */
struct qemuDomainJobSignalsData jobSignalsData; /* Signal specific data */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ccaae66..2bd4d0b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5205,15 +5205,6 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
-if (qemuDomainObjBeginJob(vm) 0)
-goto cleanup;
-
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-%s, _(domain is not running));
-goto endjob;
-}
-
for (i = 0 ; i vm-def-ndisks ; i++) {
if
[libvirt] [PATCH] Remove obsolete remoteDispatchOOMError method
No new code should be using remoteDispatchOOMError()
* daemon/dispatch.c, daemon/dispatch.h: Remove remoteDispatchOOMError
---
daemon/dispatch.c |8
daemon/dispatch.h |1 -
2 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/daemon/dispatch.c b/daemon/dispatch.c
index 7453451..010be1e 100644
--- a/daemon/dispatch.c
+++ b/daemon/dispatch.c
@@ -104,14 +104,6 @@ void remoteDispatchGenericError (remote_error *rerr)
}
-void remoteDispatchOOMError (remote_error *rerr)
-{
-remoteDispatchStringError(rerr,
- VIR_ERR_NO_MEMORY,
- out of memory);
-}
-
-
void remoteDispatchError(remote_error *rerr)
{
virErrorPtr verr = virGetLastError();
diff --git a/daemon/dispatch.h b/daemon/dispatch.h
index 79d35ac..f24f494 100644
--- a/daemon/dispatch.h
+++ b/daemon/dispatch.h
@@ -45,7 +45,6 @@ void remoteDispatchFormatError (remote_error *rerr,
void remoteDispatchAuthError (remote_error *rerr);
void remoteDispatchGenericError (remote_error *rerr);
-void remoteDispatchOOMError (remote_error *rerr);
void remoteDispatchError(remote_error *rerr);
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove obsolete remoteDispatchOOMError method
On 05/16/2011 11:28 AM, Daniel P. Berrange wrote: No new code should be using remoteDispatchOOMError() * daemon/dispatch.c, daemon/dispatch.h: Remove remoteDispatchOOMError --- daemon/dispatch.c |8 daemon/dispatch.h |1 - 2 files changed, 0 insertions(+), 9 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix leak of mime type string in screenshot dispatcher
On 05/16/2011 11:29 AM, Daniel P. Berrange wrote:
* daemon/remote.c: Free mime string
---
daemon/remote.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index bb01c89..ea36bf5 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1746,6 +1746,7 @@ remoteDispatchDomainScreenshot(struct qemud_server
*server ATTRIBUTE_UNUSED,
cleanup:
if (rv 0)
remoteDispatchError(rerr);
+VIR_FREE(mime);
if (dom)
virDomainFree(dom);
if (stream rv != 0) {
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Change some alignments in the input file
On 05/14/2011 04:34 AM, Matthias Bolte wrote: 2011/5/1 Matthias Bolte matthias.bo...@googlemail.com: No functional change included. --- src/esx/esx_vi_generator.input | 122 1 files changed, 61 insertions(+), 61 deletions(-) -method RetrieveProperties returns ObjectContent ol -ManagedObjectReference _this:PropertyCollector r +method RetrievePropertiesreturns ObjectContent ol +ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end The s/_this:PropertyCollector/_this:propertyCollector/ change should have been in the previous commit. So here a fixed v2. ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Change generated method parameter autobinding
On 05/14/2011 04:31 AM, Matthias Bolte wrote: 2011/5/13 Eric Blake ebl...@redhat.com: On 05/01/2011 01:57 PM, Matthias Bolte wrote: Instead of specifing the type of the managed object directly specify the ServiceContent member name. This way the mapping dictionary can be removed. This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2. CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector' One line of change that should have been in this patch slipped into the next one :( Here's a v2 that fixes this. ACK to v2. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Whitespace cleanup in the generator
On 05/01/2011 01:57 PM, Matthias Bolte wrote: Break long lines and change spacing of keyword arguments to match Python style standards better. No functional change included. --- src/esx/esx_vi_generator.py | 369 -- 1 files changed, 246 insertions(+), 123 deletions(-) I don't know the python style standards, but I could at least review this on the grounds of looking like safe whitespace changes. ACK. @@ -830,7 +908,8 @@ class Object(Type): if self.features Object.FEATURE__LIST: source += /* esxVI_%s_SerializeList */\n % self.name -source += ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n % self.name +source += ESX_VI__TEMPLATE__LIST__SERIALIZE(%s)\n\n \ + % self.name # deserilaize You may want to do a followup patch to fix typos like this. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix race condition in esxVI_EnsureSession
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
When the session has expired then multiple threads can race while
reestablishing it.
This race condition is not that critical as it requires a special usage
pattern to be triggerd. It can only happen when an application doesn't
s/triggerd/triggered/
do API calls for quite some time (the session expires after 30 min
inactivity) and then multiple threads doing simultaneous API calls and
end up doing simultaneous calls to esxVI_EnsureSession.
---
src/esx/esx_vi.c | 47 +++
src/esx/esx_vi.h |4 +++-
2 files changed, 38 insertions(+), 13 deletions(-)
C code - I'm back in my element!
+++ b/src/esx/esx_vi.h
@@ -185,6 +185,7 @@ int esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared,
esxVI_CURL *curl);
*/
struct _esxVI_Context {
+/* All members are used read-only after esxVI_Context_Connect ... */
esxVI_CURL *curl;
char *url;
char *ipAddress;
@@ -193,7 +194,8 @@ struct _esxVI_Context {
esxVI_ServiceContent *service;
esxVI_APIVersion apiVersion;
esxVI_ProductVersion productVersion;
-esxVI_UserSession *session;
+esxVI_UserSession *session; /* ... except the session ... */
+virMutexPtr sessionLock; /* ... that is protected by this mutex */
ACK, and thanks for those comments.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Convert autoAnswer from esxVI_Boolean to a simple bool
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
Just true/false is good enough for it. Also use it directly from the
parsed URI instead of caching it in esxPrivate.
---
src/esx/esx_driver.c | 52 -
src/esx/esx_private.h|1 -
src/esx/esx_storage_driver.c | 12 ++---
src/esx/esx_vi.c | 15 +--
src/esx/esx_vi.h |8 +++---
5 files changed, 44 insertions(+), 44 deletions(-)
+++ b/src/esx/esx_private.h
@@ -41,7 +41,6 @@ typedef struct _esxPrivate {
int32_t maxVcpus;
esxVI_Boolean supportsVMotion;
esxVI_Boolean supportsLongMode; /* aka x86_64 */
-esxVI_Boolean autoAnswer;
int32_t usedCpuTimeCounterId;
} esxPrivate;
+++ b/src/esx/esx_vi.c
@@ -2482,7 +2482,7 @@ int
esxVI_LookupVirtualMachineByUuidAndPrepareForTask
(esxVI_Context *ctx, const unsigned char *uuid,
esxVI_String *propertyNameList, esxVI_ObjectContent **virtualMachine,
- esxVI_Boolean autoAnswer)
+ bool autoAnswer)
Everything else is fallout from these two categories of changes (more
than one method signature affected, and lots of callers adjusted).
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Simplify some esxVI_Boolean to bool
On 05/01/2011 01:57 PM, Matthias Bolte wrote:
---
src/esx/esx_driver.c |6 ++
src/esx/esx_vi.c | 34 +++---
src/esx/esx_vi.h | 12 +---
3 files changed, 22 insertions(+), 30 deletions(-)
I got a trivial rebase error when testing this out:
@@ -3735,13 +3732,12 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
}
if (taskInfo-cancelable == esxVI_Boolean_True) {
-if (esxVI_CancelTask(ctx, task) 0
-blocked == esxVI_Boolean_True) {
+if (esxVI_CancelTask(ctx, task) 0 blocked) {
VIR_ERROR0(_(Cancelable task is blocked by an
now that the context has VIR_ERROR instead of VIR_ERROR0. But I'm
assuming you've already picked up on that.
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] remote generator: Add special case for virConnectGetType
This patch depends on this unapplied series:
https://www.redhat.com/archives/libvir-list/2011-May/msg00987.html
Matthias
From 33fbe44130aa055d4164f4cc1ace3dabdcfaa7d5 Mon Sep 17 00:00:00 2001
From: Matthias Bolte matthias.bo...@googlemail.com
Date: Mon, 16 May 2011 20:10:06 +0200
Subject: [PATCH] remote generator: Add special case for virConnectGetType
---
daemon/remote.c | 35 ---
daemon/remote_generator.pl | 17 +++--
src/remote/remote_protocol.x |2 +-
3 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index c98e80c..db20557 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -460,41 +460,6 @@ remoteDispatchClose(struct qemud_server *server ATTRIBUTE_UNUSED,
}
static int
-remoteDispatchGetType(struct qemud_server *server ATTRIBUTE_UNUSED,
- struct qemud_client *client ATTRIBUTE_UNUSED,
- virConnectPtr conn,
- remote_message_header *hdr ATTRIBUTE_UNUSED,
- remote_error *rerr,
- void *args ATTRIBUTE_UNUSED, remote_get_type_ret *ret)
-{
-const char *type;
-int rv = -1;
-
-if (!conn) {
-virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
-goto cleanup;
-}
-
-if (!(type = virConnectGetType(conn)))
-goto cleanup;
-
-/* We have to strdup because remoteDispatchClientRequest will
- * free this string after it's been serialised.
- */
-if (!(ret-type = strdup(type))) {
-virReportOOMError();
-goto cleanup;
-}
-
-rv = 0;
-
-cleanup:
-if (rv 0)
-remoteDispatchError(rerr);
-return rv;
-}
-
-static int
remoteDispatchDomainGetSchedulerType(struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client ATTRIBUTE_UNUSED,
virConnectPtr conn,
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index 5725583..2976c0f 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -436,8 +436,21 @@ elsif ($opt_b) {
unshift(@args_list, $conn);
}
} elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
-push(@vars_list, char *$1);
-push(@ret_list, ret-$1 = $1;);
+if ($call-{ProcName} eq GetType) {
+# SPECIAL: virConnectGetType returns a constant string that must
+# not be freed. Therefore, duplicate the string here.
+push(@vars_list, const char *$1);
+push(@ret_list, /* We have to strdup because remoteDispatchClientRequest will);
+push(@ret_list, * free this string after it's been serialised. */);
+push(@ret_list, if (!(ret-type = strdup(type))) {);
+push(@ret_list, virReportOOMError(););
+push(@ret_list, goto cleanup;);
+push(@ret_list, });
+} else {
+push(@vars_list, char *$1);
+push(@ret_list, ret-$1 = $1;);
+}
+
$single_ret_var = $1;
$single_ret_by_ref = 0;
$single_ret_check = == NULL;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 34cdce4..5c89f8f 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2051,7 +2051,7 @@ enum remote_procedure {
* view. A readstream transfers data from daemon to src/remote. */
REMOTE_PROC_OPEN = 1, /* skipgen skipgen */
REMOTE_PROC_CLOSE = 2, /* skipgen skipgen */
-REMOTE_PROC_GET_TYPE = 3, /* skipgen skipgen */
+REMOTE_PROC_GET_TYPE = 3, /* autogen skipgen */
REMOTE_PROC_GET_VERSION = 4, /* autogen autogen */
REMOTE_PROC_GET_MAX_VCPUS = 5, /* autogen autogen */
REMOTE_PROC_NODE_GET_INFO = 6, /* autogen autogen */
--
1.7.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Change generated method parameter autobinding
2011/5/16 Eric Blake ebl...@redhat.com: On 05/14/2011 04:31 AM, Matthias Bolte wrote: 2011/5/13 Eric Blake ebl...@redhat.com: On 05/01/2011 01:57 PM, Matthias Bolte wrote: Instead of specifing the type of the managed object directly specify the ServiceContent member name. This way the mapping dictionary can be removed. This fell apart for me; I don't know how to fix it, so from this point on, you'll have to rebase and post v2. CC libvirt_driver_esx_la-esx_vi_methods.lo In file included from esx/esx_vi_methods.c:288:0: esx/esx_vi_methods.generated.c: In function 'esxVI_RetrieveProperties': esx/esx_vi_methods.generated.c:520:278: error: 'esxVI_ServiceContent' has no member named 'PropertyCollector' One line of change that should have been in this patch slipped into the next one :( Here's a v2 that fixes this. ACK to v2. Thanks, pushed v2. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Change some alignments in the input file
2011/5/16 Eric Blake ebl...@redhat.com: On 05/14/2011 04:34 AM, Matthias Bolte wrote: 2011/5/1 Matthias Bolte matthias.bo...@googlemail.com: No functional change included. --- src/esx/esx_vi_generator.input | 122 1 files changed, 61 insertions(+), 61 deletions(-) -method RetrieveProperties returns ObjectContent ol - ManagedObjectReference _this:PropertyCollector r +method RetrieveProperties returns ObjectContent ol + ManagedObjectReference _this:propertyCollector r PropertyFilterSpec specSet rl end The s/_this:PropertyCollector/_this:propertyCollector/ change should have been in the previous commit. So here a fixed v2. ACK. Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: drop unused sys/syslimits.h header
sys/syslimits.h is not standardized, so portable programs should not need to rely on it. If there really is something that we need where sys/syslimits.h provided the limit but limits.h did not, then that would be a candidate for fixing in gnulib. But this patch did not turn up any compilation failures on Linux. * src/internal.h (includes): Drop unused header. * daemon/libvirtd.h (includes): Likewise. * configure.ac (AC_CHECK_HEADERS): Likewise. Based on a report by Matthias Bolte. --- configure.ac |2 +- daemon/libvirtd.h |4 src/internal.h|4 3 files changed, 1 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 4f5c2d7..233e4af 100644 --- a/configure.ac +++ b/configure.ac @@ -133,7 +133,7 @@ AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init]) LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). -AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/un.h \ +AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h]) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index d37c3fd..ea00d5c 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -38,10 +38,6 @@ # include dbus/dbus.h # endif -# ifdef HAVE_SYS_SYSLIMITS_H -# include sys/syslimits.h -# endif - # include rpc/types.h # include rpc/xdr.h # include remote_protocol.h diff --git a/src/internal.h b/src/internal.h index 0fa097c..5747a90 100644 --- a/src/internal.h +++ b/src/internal.h @@ -18,10 +18,6 @@ # define sa_assert(expr) /* empty */ # endif -# ifdef HAVE_SYS_SYSLIMITS_H -# include sys/syslimits.h -# endif - /* The library itself is allowed to use deprecated functions / * variables, so effectively undefine the deprecated attribute * which would otherwise be defined in libvirt.h. -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] remote generator, client: Handle functions that return an optional string
On 05/15/2011 12:23 AM, Matthias Bolte wrote:
---
daemon/remote_generator.pl |5 +
src/remote/remote_driver.c | 26 --
src/remote/remote_protocol.x |2 +-
3 files changed, 6 insertions(+), 27 deletions(-)
To test this, I installed the patch, and compared
src/remote/remote_client_bodies.h with the hand-maintained code being
removed; the only differences were in whitespace or trivial declaration
reordering.
ACK.
+++ b/src/remote/remote_protocol.x
@@ -2091,7 +2091,7 @@ enum remote_procedure {
REMOTE_PROC_NODE_LIST_DEVICES = 112, /* autogen autogen */
REMOTE_PROC_NODE_DEVICE_LOOKUP_BY_NAME = 113, /* autogen autogen */
REMOTE_PROC_NODE_DEVICE_GET_XML_DESC = 114, /* autogen autogen */
-REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115, /* skipgen skipgen */
+REMOTE_PROC_NODE_DEVICE_GET_PARENT = 115, /* skipgen autogen */
REMOTE_PROC_NODE_DEVICE_NUM_OF_CAPS = 116, /* autogen autogen */
REMOTE_PROC_NODE_DEVICE_LIST_CAPS = 117, /* autogen autogen */
REMOTE_PROC_NODE_DEVICE_DETTACH = 118, /* autogen skipgen */
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] remote generator, client: Add more special case handling
On 05/15/2011 12:23 AM, Matthias Bolte wrote:
For virDomainDestroy and virDrvSupportsFeature.
---
daemon/remote_generator.pl | 14 ++
src/remote/remote_driver.c | 55
--
src/remote/remote_protocol.x |4 +-
3 files changed, 16 insertions(+), 57 deletions(-)
Comparison of the new generated functions to the replaced versions is
sane (not identical, but the differences do not affect operation).
ACK.
+++ b/src/remote/remote_protocol.x
@@ -1978,7 +1978,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_CREATE_XML = 10, /* autogen autogen */
REMOTE_PROC_DOMAIN_DEFINE_XML = 11, /* autogen autogen */
-REMOTE_PROC_DOMAIN_DESTROY = 12, /* autogen skipgen */
+REMOTE_PROC_DOMAIN_DESTROY = 12, /* autogen autogen */
REMOTE_PROC_DOMAIN_DETACH_DEVICE = 13, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_XML_DESC = 14, /* autogen autogen */
REMOTE_PROC_DOMAIN_GET_AUTOSTART = 15, /* autogen autogen */
@@ -2030,7 +2030,7 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_GET_SCHEDULER_PARAMETERS = 57, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS = 58, /* skipgen skipgen */
REMOTE_PROC_GET_HOSTNAME = 59, /* autogen autogen */
-REMOTE_PROC_SUPPORTS_FEATURE = 60, /* autogen skipgen */
+REMOTE_PROC_SUPPORTS_FEATURE = 60, /* autogen autogen */
REMOTE_PROC_DOMAIN_MIGRATE_PREPARE = 61, /* skipgen skipgen */
REMOTE_PROC_DOMAIN_MIGRATE_PERFORM = 62, /* autogen autogen */
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] Fix error reporting in stream creation code
On 05/15/2011 12:23 AM, Matthias Bolte wrote: virStreamNew needs to dispatch the error that virGetStream reports on failure. remoteCreateClientStream can fail due to virStreamNew or due to VIR_ALLOC. Report OOM error for VIR_ALLOC failure to report errors in all error cases. Remove OOM error reporting from remoteCreateClientStream callers. --- daemon/remote.c |8 ++-- daemon/stream.c |7 ++- src/libvirt.c |2 ++ 3 files changed, 10 insertions(+), 7 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] remote generator: Handle stream-using functions
On 05/15/2011 12:23 AM, Matthias Bolte wrote:
Reorder signature of virDrvDomainOpenConsole to match the common pattern.
Add special case code to handle deviation in the public API version.
Adds a missing remoteStreamRelease to remoteDomainScreenshot error path.
---
daemon/remote.c | 181
daemon/remote_generator.pl | 104 +++--
src/driver.h |2 +-
src/libvirt.c|2 +-
src/lxc/lxc_driver.c |2 +-
src/qemu/qemu_driver.c |2 +-
src/remote/remote_driver.c | 207
--
src/remote/remote_protocol.x | 20 +++--
src/uml/uml_driver.c |2 +-
src/xen/xen_driver.c |2 +-
10 files changed, 113 insertions(+), 411 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index e3bd4a2..f8db6fe 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1171,50 +1171,6 @@ cleanup:
}
static int
-remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server
ATTRIBUTE_UNUSED,
Hmm, your patch was written before Dan's migration v3 patch; you may
need to tweak it to cover PrepareTunnel3 in the same manner. However,
of the functions you moved, I validated that the generated version has
the same behavior as the deleted version.
+++ b/daemon/remote_generator.pl
@@ -43,7 +43,7 @@ sub name_to_ProcName {
# Read the input file (usually remote_protocol.x) and form an
# opinion about the name, args and return type of each RPC.
-my ($name, $ProcName, $id, $flags, $gen, %calls, @calls);
+my ($name, $ProcName, $id, $flags, %calls, @calls);
# only generate a close method if -c was passed
if ($opt_c) {
@@ -135,19 +135,26 @@ while (PROTOCOL) {
$ProcName = name_to_ProcName ($name);
if ($opt_b or $opt_k) {
-if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*\*\/\s*$/)) {
+if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(.*)\*\/\s*$/)) {
die invalid generator flags for ${procprefix}_PROC_${name}
}
-$gen = $opt_b ? $1 : $2;
+my $genmode = $opt_b ? $1 : $2;
+my $genflags = $3;
-if ($gen eq autogen) {
+if ($genmode eq autogen) {
push(@autogen, $ProcName);
-} elsif ($gen eq skipgen) {
+} elsif ($genmode eq skipgen) {
# ignore it
} else {
die invalid generator flags for ${procprefix}_PROC_${name}
}
+
+if (defined $genflags and $genflags =~
m/\|\s*(read|write)stream/) {
+$calls{$name}-{streamflag} = $1;
+} else {
+$calls{$name}-{streamflag} = none;
+}
Really, we have readstream, writestream, or empty. Should we make this
stricter and reject unknown words, rather than silently translating them
to none?
@@ -854,9 +908,14 @@ elsif ($opt_k) {
}
}
-if ($call-{ProcName} eq DomainMigrateSetMaxDowntime
and
-$arg_name eq downtime) {
-$type_name = unsigned long long;
+# SPECIAL: some hyper parameters map to long longs
+if (($call-{ProcName} eq DomainMigrateSetMaxDowntime
and
+ $arg_name eq downtime) or
+($call-{ProcName} eq StorageVolUpload and
+ ($arg_name eq offset or $arg_name eq length)) or
+($call-{ProcName} eq StorageVolDownload and
+ ($arg_name eq offset or $arg_name eq length))) {
+$type_name .= long;
Idea for a future patch - should we add annotations to remote_protocol.h
for which hyper fields map to 'long' (and thus need overflow checking on
32-bit hosts) and which are 'long long'? That is, instead of
special-casing per function name in remote_generator.pl, it might be
nice to annotate it directly in the .x file (and error out if you use
'[unsigned] hyper' but don't say whether it maps to 'long' or 'long long').
+++ b/src/driver.h
@@ -516,8 +516,8 @@ typedef int
typedef int
(*virDrvDomainOpenConsole)(virDomainPtr dom,
- const char *devname,
virStreamPtr st,
+ const char *devname,
unsigned int flags);
I would have preferred seeing this split into two patches; one for the
driver.h callback reorganization, and the other for the generator
functions. I think it's okay as one, but it makes this patch rather
large; especially since you have to rebase anyways for picking up
migration v3.
+++ b/src/remote/remote_protocol.x
@@ -1965,7 +1965,10 @@ const REMOTE_PROTOCOL_VERSION = 1;
enum remote_procedure {
/* Each
Re: [libvirt] [PATCH 5/5] remote generator: Don't rely on $_ being stable over a large function
On 05/15/2011 12:23 AM, Matthias Bolte wrote:
Replace $calls{$_} with $call in the dispatch bodies generator function.
No functional change included.
---
daemon/remote_generator.pl | 100 ++-
1 files changed, 51 insertions(+), 49 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index c2468b9..5725583 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -272,11 +272,13 @@ elsif ($opt_b) {
my @keys = sort (keys %calls);
foreach (@keys) {
+my $call = $calls{$_};
+
# skip things which are REMOTE_MESSAGE
-next if $calls{$_}-{msg};
+next if $call-{msg};
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote generator: Add special case for virConnectGetType
On 05/16/2011 12:14 PM, Matthias Bolte wrote:
This patch depends on this unapplied series:
https://www.redhat.com/archives/libvir-list/2011-May/msg00987.html
which is now reviewed.
Subject: [PATCH] remote generator: Add special case for virConnectGetType
---
daemon/remote.c | 35 ---
daemon/remote_generator.pl | 17 +++--
src/remote/remote_protocol.x |2 +-
3 files changed, 16 insertions(+), 38 deletions(-)
+++ b/src/remote/remote_protocol.x
@@ -2051,7 +2051,7 @@ enum remote_procedure {
* view. A readstream transfers data from daemon to src/remote. */
REMOTE_PROC_OPEN = 1, /* skipgen skipgen */
REMOTE_PROC_CLOSE = 2, /* skipgen skipgen */
-REMOTE_PROC_GET_TYPE = 3, /* skipgen skipgen */
+REMOTE_PROC_GET_TYPE = 3, /* autogen skipgen */
REMOTE_PROC_GET_VERSION = 4, /* autogen autogen */
REMOTE_PROC_GET_MAX_VCPUS = 5, /* autogen autogen */
REMOTE_PROC_NODE_GET_INFO = 6, /* autogen autogen */
ACK.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Improve editing
On 05/16/2011 01:12 AM, Daniel Veillard wrote: On Fri, May 13, 2011 at 02:42:29PM +0100, Daniel P. Berrange wrote: There are two common problems with virsh edit friends - Invalid XML syntax, causes error report lost changes - User add unsupported/unknown XML attributes/elements which are silently discarded by libvirt This patch only fixes the first problem. It would be nice to fix the second two, by running the XML through the RNG schema validator. Rather than do this in virsh though, I'd add some flags to the virDefine/Create APIs, eg VIR_DOMAIN_XML_VALIDATE virsh can set this flag by default, and if the XML fails validation, it could prompt the user, asking if they want to proceed anyway (in which case recall the same API but without the validate flag set), or re-edit the XML Hum, yes I agree with the option of validating on define of APIs the only problem is that we tend to have holes in the RNG, but since that would be optional I think that's okay, this would hopefully help finding the mismatches between the RNG and the C parsing code. There's also the idea of doing a round trip parse - dumpxml - compare; but that also has problems where dumpxml sometimes rearranges elements or populates backwards-compatibility additions that were not present in the original. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] FreeBSD and sysconf(_SC_GETPW_R_SIZE_MAX)
On 05/16/2011 03:27 AM, Daniel P. Berrange wrote:
On Sun, May 15, 2011 at 07:22:46AM +0200, Matthias Bolte wrote:
On FreeBSD virsh fails to enter interactive mode because
vshReadlineInit fails, because virGetUserDirectory fails, because
virGetUserEnt fails, because sysconf(_SC_GETPW_R_SIZE_MAX) returns -1
and sets errno to EINVAL.
Is this something that gnulib should/could deal with, Eric?
sysconf() is a can of worms; no chance that gnulib will touch it any
time soon.
Or should we work around it in libvrt and fallback to PATH_MAX when
sysconf(_SC_GETPW_R_SIZE_MAX) fails?
PATH_MAX isn't expected to be available on all platforms either which
is one of the reasons for us removing its use.
getpwnam() returns ERANGE if the allocated buffer is too small. So
we should do something along the lines of
size_t len = sysconf(_SC_GETPW_R_SIZE_MAX)
Guarded by #ifdef, and defaulting to -1 if _SC_GETPW_R_SIZE_MAX is not
defined (since it is a handy extension, but not present everywhere).
if (len 0)
len = 1024;
while (1) {
...
err = getpwnam()
if (err 0) {
if (errno == ERANGE) {
len += 1024;
Or len = 1, to avoid quadratic performance.
But indeed this is the correct response.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: tolerate unlimited group size
POSIX allows sysconf(_SC_GETPW_R_SIZE_MAX) to return -1 if there
is no fixed limit, and requires ERANGE errors to track real size.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html
Also, on error for get*_r functions, errno is undefined, and the
real error was the return value.
* src/util/util.c (virGetUserEnt, virGetUserID, virGetGroupID)
(virSetUIDGID): Cope with sysconf failure or too small buffer.
Reported by Matthias Bolte.
---
This pretty much copies the example implementation straight from POSIX
for getting a hint for initial size, then iterating until ERANGE no
longer occurs.
src/util/util.c | 74 +-
1 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index f169e54..240f776 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -603,7 +603,7 @@ virExecWithHook(const char *const*argv,
goto fork_error;
}
-openmax = sysconf (_SC_OPEN_MAX);
+openmax = sysconf(_SC_OPEN_MAX);
for (i = 3; i openmax; i++)
if (i != infd
i != null
@@ -2743,11 +2743,11 @@ static char *virGetUserEnt(uid_t uid,
struct passwd *pw = NULL;
long val = sysconf(_SC_GETPW_R_SIZE_MAX);
size_t strbuflen = val;
+int rc;
-if (val 0) {
-virReportSystemError(errno, %s, _(sysconf failed));
-return NULL;
-}
+/* sysconf is a hint; if it fails, fall back to a reasonable size */
+if (val 0)
+strbuflen = 1024;
if (VIR_ALLOC_N(strbuf, strbuflen) 0) {
virReportOOMError();
@@ -2761,8 +2761,15 @@ static char *virGetUserEnt(uid_t uid,
* 0 or ENOENT or ESRCH or EBADF or EPERM or ...
*The given name or uid was not found.
*/
-if (getpwuid_r(uid, pwbuf, strbuf, strbuflen, pw) != 0 || pw == NULL) {
-virReportSystemError(errno,
+while ((rc = getpwuid_r(uid, pwbuf, strbuf, strbuflen, pw)) == ERANGE) {
+if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) 0) {
+virReportOOMError();
+VIR_FREE(strbuf);
+return NULL;
+}
+}
+if (rc != 0 || pw == NULL) {
+virReportSystemError(rc,
_(Failed to find user record for uid '%u'),
(unsigned int) uid);
VIR_FREE(strbuf);
@@ -2800,11 +2807,11 @@ int virGetUserID(const char *name,
struct passwd *pw = NULL;
long val = sysconf(_SC_GETPW_R_SIZE_MAX);
size_t strbuflen = val;
+int rc;
-if (val 0) {
-virReportSystemError(errno, %s, _(sysconf failed));
-return -1;
-}
+/* sysconf is a hint; if it fails, fall back to a reasonable size */
+if (val 0)
+strbuflen = 1024;
if (VIR_ALLOC_N(strbuf, strbuflen) 0) {
virReportOOMError();
@@ -2818,8 +2825,15 @@ int virGetUserID(const char *name,
* 0 or ENOENT or ESRCH or EBADF or EPERM or ...
*The given name or uid was not found.
*/
-if (getpwnam_r(name, pwbuf, strbuf, strbuflen, pw) != 0 || pw == NULL) {
-virReportSystemError(errno,
+while ((rc = getpwnam_r(name, pwbuf, strbuf, strbuflen, pw)) == ERANGE) {
+if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) 0) {
+virReportOOMError();
+VIR_FREE(strbuf);
+return -1;
+}
+}
+if (rc != 0 || pw == NULL) {
+virReportSystemError(rc,
_(Failed to find user record for name '%s'),
name);
VIR_FREE(strbuf);
@@ -2842,11 +2856,11 @@ int virGetGroupID(const char *name,
struct group *gr = NULL;
long val = sysconf(_SC_GETGR_R_SIZE_MAX);
size_t strbuflen = val;
+int rc;
-if (val 0) {
-virReportSystemError(errno, %s, _(sysconf failed));
-return -1;
-}
+/* sysconf is a hint; if it fails, fall back to a reasonable size */
+if (val 0)
+strbuflen = 1024;
if (VIR_ALLOC_N(strbuf, strbuflen) 0) {
virReportOOMError();
@@ -2860,8 +2874,15 @@ int virGetGroupID(const char *name,
* 0 or ENOENT or ESRCH or EBADF or EPERM or ...
*The given name or uid was not found.
*/
-if (getgrnam_r(name, grbuf, strbuf, strbuflen, gr) != 0 || gr == NULL) {
-virReportSystemError(errno,
+while ((rc = getgrnam_r(name, grbuf, strbuf, strbuflen, gr)) == ERANGE) {
+if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) 0) {
+virReportOOMError();
+VIR_FREE(strbuf);
+return -1;
+}
+}
+if (rc != 0 || gr == NULL) {
+virReportSystemError(rc,
_(Failed to find group record for name '%s'),
name);
VIR_FREE(strbuf);
@@ -2897,6 +2918,7 @@ virSetUIDGID(uid_t uid, gid_t gid)
struct passwd pwd, *pwd_result;
char *buf = NULL;
Re: [libvirt] FreeBSD and sysconf(_SC_GETPW_R_SIZE_MAX)
On 05/16/2011 03:03 PM, Eric Blake wrote: getpwnam() returns ERANGE if the allocated buffer is too small. So we should do something along the lines of size_t len = sysconf(_SC_GETPW_R_SIZE_MAX) Guarded by #ifdef, and defaulting to -1 if _SC_GETPW_R_SIZE_MAX is not defined (since it is a handy extension, but not present everywhere). Or not - we only use it inside #if HAVE_FUNC_GETPWNAM_R and friends; so far, all platforms that have the *_r functions also happen to have a decent set of sysconf() names where we aren't running into compilation errors by trying to use _SC_GETPW_R_SIZE_MAX. But indeed this is the correct response. I've posted https://www.redhat.com/archives/libvir-list/2011-May/msg01074.html -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] send 'container=libvirt' in env to container init
On 05/16/2011 09:14 AM, Serge Hallyn wrote: @@ -112,6 +112,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=libvirt); POSIX reserves lowercase env names for the user. Is upstart really using a lower case name, or should this be an upper case name? No upstart isn't doing anything itself :) Just blame me - the upstart scripts I put into lxcguest do in fact check for lower-case. What do you mean by 'for the user'? For the user to type into the boot args, or something else? http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html Environment variable names used by the utilities in the Shell and Utilities volume of POSIX.1-2008 consist solely of uppercase letters, digits, and the underscore ( '_' ) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation; applications shall tolerate the presence of such names. Uppercase and lowercase letters shall retain their unique identities and shall not be folded together. The name space of environment variable names containing lowercase letters is reserved for applications. Applications can define any environment variables with names from this name space without modifying the behavior of the standard utilities. That is, if your startup scripts make anything in the system, including /bin/sh, behave differently because it is in a container, then it's probably better to use an upper-case name, since /bin/sh should not behave differently regardless of what lower-case names are in the environment. But this is a borderline case - upstart is outside the realm of POSIX, and you can easily arrange for $container to be reset before any standard applications are ever invoked from within the container. virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); Can upstart be taught to look for LIBIVRT_LXC_UUID instead? That's a possibility, yes. It just makes it less symmetric to the liblxc case, but if you prefer that I see no reason why it wouldn't work. Do you have a pointer to the liblxc code that's inspecting its environment? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Improve editing
On Mon, May 16, 2011 at 02:59:28PM -0600, Eric Blake wrote: On 05/16/2011 01:12 AM, Daniel Veillard wrote: On Fri, May 13, 2011 at 02:42:29PM +0100, Daniel P. Berrange wrote: There are two common problems with virsh edit friends - Invalid XML syntax, causes error report lost changes - User add unsupported/unknown XML attributes/elements which are silently discarded by libvirt This patch only fixes the first problem. It would be nice to fix the second two, by running the XML through the RNG schema validator. Rather than do this in virsh though, I'd add some flags to the virDefine/Create APIs, eg VIR_DOMAIN_XML_VALIDATE virsh can set this flag by default, and if the XML fails validation, it could prompt the user, asking if they want to proceed anyway (in which case recall the same API but without the validate flag set), or re-edit the XML Hum, yes I agree with the option of validating on define of APIs the only problem is that we tend to have holes in the RNG, but since that would be optional I think that's okay, this would hopefully help finding the mismatches between the RNG and the C parsing code. There's also the idea of doing a round trip parse - dumpxml - compare; but that also has problems where dumpxml sometimes rearranges elements or populates backwards-compatibility additions that were not present in the original. Yeah that can't fly, even for an XML/Markup editor preserving this kind of non-strucural information is nearly impossible, since we even discard the tree in libvirt after parsing we really can't hope to do anything like 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
Re: [libvirt] [PATCH] send 'container=libvirt' in env to container init
Quoting Eric Blake (ebl...@redhat.com): On 05/16/2011 09:14 AM, Serge Hallyn wrote: @@ -112,6 +112,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) virCommandAddEnvString(cmd, PATH=/bin:/sbin); virCommandAddEnvString(cmd, TERM=linux); +virCommandAddEnvString(cmd, container=libvirt); POSIX reserves lowercase env names for the user. Is upstart really using a lower case name, or should this be an upper case name? No upstart isn't doing anything itself :) Just blame me - the upstart scripts I put into lxcguest do in fact check for lower-case. What do you mean by 'for the user'? For the user to type into the boot args, or something else? http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html Environment variable names used by the utilities in the Shell and Utilities volume of POSIX.1-2008 consist solely of uppercase letters, digits, and the underscore ( '_' ) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation; applications shall tolerate the presence of such names. Uppercase and lowercase letters shall retain their unique identities and shall not be folded together. The name space of environment variable names containing lowercase letters is reserved for applications. Applications can define any environment variables with names from this name space without modifying the behavior of the standard utilities. That is, if your startup scripts make anything in the system, including /bin/sh, behave differently because it is in a container, then it's Ah, this is about 'proper' environment variables. But environment variables are just being used to pass name=value boot arguments to init. So I wouldn't have thought this would apply. It would apply from the point of view of libvirt starting a program, but not from the point of view of init trying to parse its command line arguments. Very interesting. When you consider the init=/bin/sh case, I can see where it would make sense. probably better to use an upper-case name, since /bin/sh should not behave differently regardless of what lower-case names are in the environment. But this is a borderline case - upstart is outside the realm of POSIX, and you can easily arrange for $container to be reset before any standard applications are ever invoked from within the container. virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr); virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name); Can upstart be taught to look for LIBIVRT_LXC_UUID instead? That's a possibility, yes. It just makes it less symmetric to the liblxc case, but if you prefer that I see no reason why it wouldn't work. Do you have a pointer to the liblxc code that's inspecting its environment? Not liblxc - liblxc just sends 'container=lxc' to init. One (of two) of the upstart scripts which use that is here: http://bazaar.launchpad.net/~serge-hallyn/ubuntu/natty/lxc/lxc-clone/view/head:/debian/lxcguest.lxcmount.upstart thanks, -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 2/6] virNodeGetCPUTimeParameters: Define internal driver API
Hi, Daniel. Thank you for your rapid reply. On Mon, 16 May 2011 18:23:07 +0100 Daniel P. Berrange berra...@redhat.com wrote: On Mon, May 16, 2011 at 11:36:14AM +0900, Minoru Usui wrote: virNodeGetCPUTimeParameters: Define internal driver API Signed-off-by: Minoru Usui u...@mxm.nes.nec.co.jp --- src/driver.h |8 src/esx/esx_driver.c |1 + src/libxl/libxl_driver.c |1 + src/lxc/lxc_driver.c |1 + src/openvz/openvz_driver.c |1 + src/phyp/phyp_driver.c |1 + src/qemu/qemu_driver.c |1 + src/remote/remote_driver.c |1 + src/test/test_driver.c |1 + src/uml/uml_driver.c |1 + src/vbox/vbox_tmpl.c |1 + src/vmware/vmware_driver.c |1 + src/xen/xen_driver.c |1 + src/xenapi/xenapi_driver.c |1 + 14 files changed, 21 insertions(+), 0 deletions(-) Unfortunately, this patch will need updating again to rebase to last GIT. Essentially you can remove all the changes in this patch, except for the one against src/driver.h OK. I'll change it. -- Minoru Usui u...@mxm.nes.nec.co.jp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 4/6] virNodeGetCPUTimeParameters: Implement remote protocol
On Mon, 16 May 2011 18:26:22 +0100
Daniel P. Berrange berra...@redhat.com wrote:
On Mon, May 16, 2011 at 11:37:34AM +0900, Minoru Usui wrote:
virNodeGetCPUTimeParameters: Implement remote protocol
Signed-off-by: Minoru Usui u...@mxm.nes.nec.co.jp
---
daemon/remote.c | 81
++
src/remote/remote_driver.c | 64 +
src/remote/remote_protocol.x | 21 ++-
3 files changed, 165 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 6e13958..f7b84d9 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1749,6 +1749,87 @@ cleanup:
return rv;
}
+static int
+remoteDispatchNodeGetCPUTimeParameters (struct qemud_server *server
+ATTRIBUTE_UNUSED,
+struct qemud_client *client
+ATTRIBUTE_UNUSED,
+virConnectPtr conn,
+remote_message_header *
+hdr ATTRIBUTE_UNUSED,
+remote_error *rerr,
+
remote_node_get_cpu_time_parameters_args *
+args,
+
remote_node_get_cpu_time_parameters_ret *
+ret)
This is horribly whitespace mangled. Please put the variable names +
annotations
on the same line as the corresponding data types.
OK.
I'll change it.
+{
+virCPUTimeParameterPtr params = NULL;
+int i;
+int nparams = args-nparams;
+unsigned int flags;
+int rv = -1;
+
+if (!conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not
open));
+goto cleanup;
+}
+
+flags = args-flags;
+
+if (nparams REMOTE_NODE_CPU_TIME_PARAMETERS_MAX) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large));
+goto cleanup;
+}
+if (VIR_ALLOC_N(params, nparams) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (virNodeGetCPUTimeParameters(conn, params, nparams, flags) 0)
+goto cleanup;
+
+/* In this case, we need to send back the number of parameters
+ * supported
+ */
+if (args-nparams == 0) {
+ret-nparams = nparams;
+goto success;
+}
+
+/* Serialise the memory parameters. */
+ret-params.params_len = nparams;
+if (VIR_ALLOC_N(ret-params.params_val, nparams) 0)
+goto no_memory;
+
+for (i = 0; i nparams; ++i) {
+/* remoteDispatchClientRequest will free this: */
+ret-params.params_val[i].field = strdup(params[i].field);
+if (ret-params.params_val[i].field == NULL)
+goto no_memory;
+
+ret-params.params_val[i].value = params[i].value;
+}
+
+success:
+rv = 0;
+
+cleanup:
+if (rv 0) {
+remoteDispatchError(rerr);
+if (ret-params.params_val) {
+for (i = 0; i nparams; i++)
+VIR_FREE(ret-params.params_val[i].field);
+VIR_FREE(ret-params.params_val);
+}
+}
+VIR_FREE(params);
+return rv;
+
+no_memory:
+virReportOOMError();
+goto cleanup;
+}
+
/*-*/
static int
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index f61afdc..46130ac 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1724,6 +1724,70 @@ done:
}
static int
+remoteNodeGetCPUTimeParameters (virConnectPtr conn,
+virCPUTimeParameterPtr params, int
*nparams,
+unsigned int flags)
+{
+int rv = -1;
+remote_node_get_cpu_time_parameters_args args;
+remote_node_get_cpu_time_parameters_ret ret;
+int i = -1;
+struct private_data *priv = conn-privateData;
+
+remoteDriverLock(priv);
+
+args.nparams = *nparams;
+args.flags = flags;
+
+memset (ret, 0, sizeof ret);
+if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_CPU_TIME_PARAMETERS,
+ (xdrproc_t) xdr_remote_node_get_cpu_time_parameters_args,
+ (char *) args,
+ (xdrproc_t) xdr_remote_node_get_cpu_time_parameters_ret,
+ (char *) ret) == -1)
+goto done;
+
+/* Check the length of the returned list carefully. */
+if (ret.params.params_len REMOTE_NODE_CPU_TIME_PARAMETERS_MAX ||
+ret.params.params_len *nparams) {
+remoteError(VIR_ERR_RPC, %s,
+_(remoteNodeGetCPUTimeParameters:
+
