Re: [libvirt] Initial working Mac OS X libvirt client build
On Mon, Oct 25, 2010 at 07:09, Justin Clift jcl...@redhat.com wrote:
On 10/25/2010 12:18 PM, Ruben Kerkhof wrote:
On Sun, Oct 24, 2010 at 02:46, Justin Clift jcl...@redhat.com wrote:
On 10/24/2010 09:33 AM, Ruben Kerkhof wrote:
snip
None at all, actually. I just started libvirtd on my local mac on
which I also have VirtualBox installed.
Speaking of which, it would be nice to have a launchctl file for
libvirtd. I might be able to come up with something...
Please do. It'd be nice to have that part working out of the box for
people as well. :)
For that to work, I'd like to run libvirtd as my own user, so I can
add the launchtl file to my own Library directory.
I'm curious, can you successfully run libvirtd as your own user (no sudo)?
03:10:17.562: error : qemudListenUnix:582 : Failed to bind socket to
'@/Users/ruben/.libvirt/libvirt-sock': No such file or directory
Actually, that looks familiar. I think I tried the same thing, but was
ok running it as root instead after getting the same error.
I didn't look into it any more though. ;)
Stepping through the code now, I see 2 (possible) issues:
First: qemudInitPaths doesn't seem to create the ~/.libvirt directory
Second: in qemudListenUnix, this piece of code:
addr.sun_family = AF_UNIX;
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
if (addr.sun_path[0] == '@')
addr.sun_path[0] = '\0';
So the first byte of the sun_path is '\0', something that Leopard
doesn't seem to like.
Breaking into gdb and setting the path manually to
/Users/ruben/.libvirt/libvirt-sock seems to work.
Interesting. We can definitely pull together a temporary OSX workaround
patch for the moment (purely in the Homebrew formula). But it would be
better to have a proper fix in libvirt instead.
How good is your C coding? :)
Terrible ;) I think the easiest fix is
if (addr[0] == '@')
addr[0] = '\0';
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
Or am I missing something?
I haven't been able to bootstrap a build from libvirt git yet, mainly
gettext issues.
Otherwise I would have come up with a proper patch.
Thanks,
Ruben
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Initial working Mac OS X libvirt client build
On Mon, Oct 25, 2010 at 10:20, Ruben Kerkhof ru...@rubenkerkhof.com wrote:
On Mon, Oct 25, 2010 at 07:09, Justin Clift jcl...@redhat.com wrote:
On 10/25/2010 12:18 PM, Ruben Kerkhof wrote:
On Sun, Oct 24, 2010 at 02:46, Justin Clift jcl...@redhat.com wrote:
On 10/24/2010 09:33 AM, Ruben Kerkhof wrote:
snip
None at all, actually. I just started libvirtd on my local mac on
which I also have VirtualBox installed.
Speaking of which, it would be nice to have a launchctl file for
libvirtd. I might be able to come up with something...
Please do. It'd be nice to have that part working out of the box for
people as well. :)
For that to work, I'd like to run libvirtd as my own user, so I can
add the launchtl file to my own Library directory.
I'm curious, can you successfully run libvirtd as your own user (no sudo)?
03:10:17.562: error : qemudListenUnix:582 : Failed to bind socket to
'@/Users/ruben/.libvirt/libvirt-sock': No such file or directory
Actually, that looks familiar. I think I tried the same thing, but was
ok running it as root instead after getting the same error.
I didn't look into it any more though. ;)
Stepping through the code now, I see 2 (possible) issues:
First: qemudInitPaths doesn't seem to create the ~/.libvirt directory
Second: in qemudListenUnix, this piece of code:
addr.sun_family = AF_UNIX;
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
if (addr.sun_path[0] == '@')
addr.sun_path[0] = '\0';
So the first byte of the sun_path is '\0', something that Leopard
doesn't seem to like.
Breaking into gdb and setting the path manually to
/Users/ruben/.libvirt/libvirt-sock seems to work.
Interesting. We can definitely pull together a temporary OSX workaround
patch for the moment (purely in the Homebrew formula). But it would be
better to have a proper fix in libvirt instead.
How good is your C coding? :)
Terrible ;) I think the easiest fix is
if (addr[0] == '@')
addr[0] = '\0';
Argh, I meant path[0] here of course.
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
Or am I missing something?
I haven't been able to bootstrap a build from libvirt git yet, mainly
gettext issues.
Otherwise I would have come up with a proper patch.
Thanks,
Ruben
Here's a (completely untested) patch. I will have more time tomorrow
to dig into this.
From 3fa6bcfca4bb50b18935cc4637426ef3ac3cdcbd Mon Sep 17 00:00:00 2001
From: Ruben Kerkhof ru...@tilaa.nl
Date: Mon, 25 Oct 2010 10:31:15 +0200
Subject: [PATCH] Fix binding to a unix socket on OSX
addr.sun_path doesn't like the first
byte to be NULL
Signed-off-by: Ruben Kerkhof ru...@tilaa.nl
---
daemon/libvirtd.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 8e88d05..76b8dc8 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -571,13 +571,14 @@ static int qemudListenUnix(struct qemud_server *server,
virSetNonBlock(sock-fd) 0)
goto cleanup;
+if (path[0] == '@')
+path[0] = '\0';
+
sock-addr.data.un.sun_family = AF_UNIX;
if (virStrcpyStatic(sock-addr.data.un.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
-if (sock-addr.data.un.sun_path[0] == '@')
-sock-addr.data.un.sun_path[0] = '\0';
oldgrp = getgid();
oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask);
--
1.7.3.1
Regards, Ruben
0001-Fix-binding-to-a-unix-socket-on-OSX.patch
Description: Binary data
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] configure: adds an option for not using the readline library
On Mon, Oct 25, 2010 at 07:00, Justin Clift jcl...@redhat.com wrote: On 10/25/2010 10:58 AM, Ruben Kerkhof wrote: snip I think this patch is still useful for users not using homebrew. I'm trying to do a (debug) build of libvirt from a tarball outside of homebrew. In this case the (keg only) readline isn't picked up. Even better would be an option to specify the headers/library location. What do you think? Ahhh, you mean like an override, which explicitly sets where libvirt will check for the readline headers and matching libraries? That makes sense, yeah. :) I got there in the end with ./configure CPPFLAGS='-I/usr/local/Cellar/libxml2/2.7.7/include -I/usr/local/Cellar/readline/6.1/include' LDFLAGS='-L/usr/local/Cellar/libxml2/2.7.7/lib -L/usr/local/Cellar/readline/6.1/lib' So I don't think the patch is really needed. What would be good though is a AC_CHECK_LIB(readline, rl_completion_matches This would work with readline 4.2 and higher, and fail on Leopard since the readline shim libedit provides doesn't do completion. If you're interested in hacking on that yourself, you can crack open the configure.ac file, and look at the other bits of code in there. That's how I do most of the stuff in there, by copying other bits of code and then applying logic+copious testing. :) Eric Blake (and others) are pretty good at this stuff, so they often have suggested on how to do things better too. Your kind of thing, or are you wanting me to have a go at it? :) As soon as I can do a proper bootstrap from git... (I'll happily leave it for you if you want) Regards and best wishes, Justin Clift Thanks! Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Initial working Mac OS X libvirt client build
On Mon, Oct 25, 2010 at 10:37:36AM +0200, Ruben Kerkhof wrote:
On Mon, Oct 25, 2010 at 10:20, Ruben Kerkhof ru...@rubenkerkhof.com wrote:
On Mon, Oct 25, 2010 at 07:09, Justin Clift jcl...@redhat.com wrote:
On 10/25/2010 12:18 PM, Ruben Kerkhof wrote:
On Sun, Oct 24, 2010 at 02:46, Justin Clift jcl...@redhat.com wrote:
On 10/24/2010 09:33 AM, Ruben Kerkhof wrote:
snip
None at all, actually. I just started libvirtd on my local mac on
which I also have VirtualBox installed.
Speaking of which, it would be nice to have a launchctl file for
libvirtd. I might be able to come up with something...
Please do. It'd be nice to have that part working out of the box for
people as well. :)
For that to work, I'd like to run libvirtd as my own user, so I can
add the launchtl file to my own Library directory.
I'm curious, can you successfully run libvirtd as your own user (no sudo)?
03:10:17.562: error : qemudListenUnix:582 : Failed to bind socket to
'@/Users/ruben/.libvirt/libvirt-sock': No such file or directory
Actually, that looks familiar. I think I tried the same thing, but was
ok running it as root instead after getting the same error.
I didn't look into it any more though. ;)
Stepping through the code now, I see 2 (possible) issues:
First: qemudInitPaths doesn't seem to create the ~/.libvirt directory
Second: in qemudListenUnix, this piece of code:
addr.sun_family = AF_UNIX;
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
if (addr.sun_path[0] == '@')
addr.sun_path[0] = '\0';
So the first byte of the sun_path is '\0', something that Leopard
doesn't seem to like.
Breaking into gdb and setting the path manually to
/Users/ruben/.libvirt/libvirt-sock seems to work.
Interesting. We can definitely pull together a temporary OSX workaround
patch for the moment (purely in the Homebrew formula). But it would be
better to have a proper fix in libvirt instead.
How good is your C coding? :)
Terrible ;) I think the easiest fix is
if (addr[0] == '@')
addr[0] = '\0';
Argh, I meant path[0] here of course.
if (virStrcpyStatic(addr.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
Or am I missing something?
I haven't been able to bootstrap a build from libvirt git yet, mainly
gettext issues.
Otherwise I would have come up with a proper patch.
Thanks,
Ruben
Here's a (completely untested) patch. I will have more time tomorrow
to dig into this.
From 3fa6bcfca4bb50b18935cc4637426ef3ac3cdcbd Mon Sep 17 00:00:00 2001
From: Ruben Kerkhof ru...@tilaa.nl
Date: Mon, 25 Oct 2010 10:31:15 +0200
Subject: [PATCH] Fix binding to a unix socket on OSX
addr.sun_path doesn't like the first
byte to be NULL
Signed-off-by: Ruben Kerkhof ru...@tilaa.nl
---
daemon/libvirtd.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 8e88d05..76b8dc8 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -571,13 +571,14 @@ static int qemudListenUnix(struct qemud_server *server,
virSetNonBlock(sock-fd) 0)
goto cleanup;
+if (path[0] == '@')
+path[0] = '\0';
+
sock-addr.data.un.sun_family = AF_UNIX;
if (virStrcpyStatic(sock-addr.data.un.sun_path, path) == NULL) {
VIR_ERROR(_(Path %s too long for unix socket), path);
goto cleanup;
}
-if (sock-addr.data.un.sun_path[0] == '@')
-sock-addr.data.un.sun_path[0] = '\0';
NACK, this results in 'path' being a zer-length string, so no data is
copied in the next virStrcpyStatic line. The original code is correctly
creating a socket in the abstract namespace, ie one which does not
appear in the filesystem
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck 1/3] Correct typos in documents
On Mon, Oct 18, 2010 at 05:45:12PM +0100, Daniel P. Berrange wrote: On Mon, Oct 18, 2010 at 07:18:07AM +0800, Osier Yang wrote: * docs/intro.pod * docs/writing-tests.pod --- docs/intro.pod | 10 +- docs/writing-tests.pod |8 2 files changed, 9 insertions(+), 9 deletions(-) ACK Okay I pushed the 5 patches ACK'ed on this thread, Thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [TCK] Add test case for domain hook testing.
On Fri, Oct 22, 2010 at 06:05:46AM +0800, Osier Yang wrote: The test case will be valid only when the Sys::Virt::TCK connection object is qemu:///system or lxc:///, because currently libvirt only support hooks for QEMU and LXC domain. Also update scripts/hooks/051-daemon-hook.t, replace $hook-foo with $hook-foo(). Okay, applied too, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck 1/3] Correct typos in documents
Thanks a lot :-) - Osier - Daniel Veillard veill...@redhat.com wrote: On Mon, Oct 18, 2010 at 05:45:12PM +0100, Daniel P. Berrange wrote: On Mon, Oct 18, 2010 at 07:18:07AM +0800, Osier Yang wrote: * docs/intro.pod * docs/writing-tests.pod --- docs/intro.pod | 10 +- docs/writing-tests.pod |8 2 files changed, 9 insertions(+), 9 deletions(-) ACK Okay I pushed the 5 patches ACK'ed on this thread, Thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Initial working Mac OS X libvirt client build
On Mon, Oct 25, 2010 at 10:48, Daniel P. Berrange berra...@redhat.com wrote: NACK, this results in 'path' being a zer-length string, so no data is copied in the next virStrcpyStatic line. The original code is correctly creating a socket in the abstract namespace, ie one which does not appear in the filesystem Daniel Ah, I didn't knew that, sorry. Am I right in assuming that an abstract namespace is a linux-only feature? The unix manpage on my Mac doesn't describe it, neither does FreeBSD (http://www.freebsd.org/cgi/man.cgi?query=unixapropos=0sektion=0manpath=FreeBSD+8.1-RELEASEformat=html). Thanks, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virsh: Add option 'model' for attach-interface
Allows to specify the NIC model type when attaching an interface,
it's an optional option.
---
tools/virsh.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3e37b06..2da9489 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -8292,6 +8292,7 @@ static const vshCmdOptDef opts_attach_interface[] = {
{target, VSH_OT_DATA, 0, N_(target network name)},
{mac,VSH_OT_DATA, 0, N_(MAC address)},
{script, VSH_OT_DATA, 0, N_(script used to bridge network interface)},
+{model, VSH_OT_DATA, 0, N_(model type)},
{persistent, VSH_OT_BOOL, 0, N_(persist interface attachment)},
{NULL, 0, 0, NULL}
};
@@ -8300,7 +8301,7 @@ static int
cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
-char *mac, *target, *script, *type, *source;
+char *mac, *target, *script, *type, *source, *model;
int typ, ret = FALSE;
unsigned int flags;
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -8319,6 +8320,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
target = vshCommandOptString(cmd, target, NULL);
mac = vshCommandOptString(cmd, mac, NULL);
script = vshCommandOptString(cmd, script, NULL);
+model = vshCommandOptString(cmd, model, NULL);
/* check interface type */
if (STREQ(type, network)) {
@@ -8345,6 +8347,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
virBufferVSprintf(buf, mac address='%s'/\n, mac);
if (script != NULL)
virBufferVSprintf(buf, script path='%s'/\n, script);
+if (model != NULL)
+virBufferVSprintf(buf, model type='%s'/\n, model);
virBufferAddLit(buf, /interface\n);
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virsh: update attach-interface docs
* tools/virsh.pod (attach-interface): add docs for new option 'model' and missed option 'persistent'. --- tools/virsh.pod |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 61875af..d904800 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -647,7 +647,7 @@ although this use only replaces the media within the existing virtual cdrom or floppy device; consider using Bupdate-device for this usage instead. Imode can specify the two specific mode Ireadonly or Ishareable. -=item Battach-interface Idomain-id Itype Isource optional I--target target I--mac mac I--script script +=item Battach-interface Idomain-id Itype Isource optional I--target target I--mac mac I--script script I--model model I--persistent Attach a new network interface to the domain. Itype can be either Inetwork to indicate a physical network device or Ibridge to indicate a bridge to a device. @@ -656,6 +656,8 @@ Itarget allows to indicate the target device in the guest. Imac allows to specify the MAC address of the network interface. Iscript allows to specify a path to a script handling a bridge instead of the default one. +Imodel allows to specify the model type. +Ipersistent indicates the changes will affect the next boot of the domain. =item Bdetach-device Idomain-id IFILE -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Initial working Mac OS X libvirt client build
On 10/25/2010 07:20 PM, Ruben Kerkhof wrote: snip I haven't been able to bootstrap a build from libvirt git yet, mainly gettext issues. What are the gettext errors that happen for you? Thinking you'll need to install a newer gettext that the OSX provided one (as Homebrew does), and then make sure the headers/libraries for it are found first. Same as you've done for the readline libraries. Something like: ./configure \ CPPFLAGS='-I/usr/local/Cellar/gettext/0.17/include -I/usr/local/Cellar/libxml2/2.7.7/include -I/usr/local/Cellar/readline/6.1/include' \ LDFLAGS='-L/usr/local/Cellar/gettext/0.17/lib -L/usr/local/Cellar/libxml2/2.7.7/lib -L/usr/local/Cellar/readline/6.1/lib' (That should actually work, as long as you have gettext 0.17 installed.) :) Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] bye to close(), welcome to VIR_(FORCE_)CLOSE()
On Mon, Oct 25, 2010 at 08:13:19AM -0400, Stefan Berger wrote:
Index: libvirt-acl/src/libvirt.c
@@ -1544,14 +1544,11 @@ cleanup:
vethDelete(veths[i]);
VIR_FREE(veths[i]);
}
-if (rc != 0 priv-monitor != -1) {
-close(priv-monitor);
-priv-monitor = -1;
-}
-if (parentTty != -1)
-close(parentTty);
-if (logfd != -1)
-close(logfd);
+if (rc != 0)
+VIR_FORCE_CLOSE(priv-monitor);
+VIR_FORCE_CLOSE(parentTty);
+if (VIR_CLOSE(logfd) 0)
+virReportSystemError(errno, %s, _(could not close logfile));
This is reporting an error without returning an error code, so the
caller will still see success.
@@ -2011,8 +2008,7 @@ lxcReconnectVM(void *payload, const char
@@ -457,11 +458,15 @@ phypUUIDTable_WriteFile(virConnectPtr co
}
}
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
return 0;
Again, reporting an error while returning success.
err:
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
return -1;
}
This is likely blowing away a previously reported error.
@@ -764,7 +769,9 @@ phypUUIDTable_Pull(virConnectPtr conn)
}
break;
}
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
goto exit;
Reporting error while returning success
@@ -6542,8 +6536,10 @@ static int qemudDomainSaveImageClose(int
{
int ret = 0;
-if (fd != -1)
-close(fd);
+if (VIR_CLOSE(fd) 0) {
+virReportSystemError(errno, %s,
+ _(cannot close file));
+}
Reporting error while returning success
Index: libvirt-acl/src/storage/storage_backend_fs.c
@@ -94,7 +95,9 @@ static int nlOpen(void)
static void nlClose(int fd)
{
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno,
+ %s,_(cannot close netlink socket));
}
No return status at all - this function likely shouldn't even
exist. Should be replaced with direct calls to VIR_FORCE_CLOSE
and VIR_CLOSE as appropriate, returning correct error codes
if it wants to handle close failures.
@@ -418,17 +419,13 @@ virHookCall(int driver, const char *id,
}
cleanup:
-if (pipefd[0] = 0) {
-if (close(pipefd[0]) 0) {
-virReportSystemError(errno, %s,
- _(unable to close pipe for hook input));
-}
-}
-if (pipefd[1] = 0) {
-if (close(pipefd[1]) 0) {
-virReportSystemError(errno, %s,
- _(unable to close pipe for hook input));
-}
+if (VIR_CLOSE(pipefd[0]) 0) {
+virReportSystemError(errno, %s,
+ _(unable to close pipe for hook input));
+}
+if (VIR_CLOSE(pipefd[1]) 0) {
+virReportSystemError(errno, %s,
+ _(unable to close pipe for hook input));
}
Reporting errors while returning success.
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Changing the default for qcow2 creation
I've recently been investigating a severe performance issue I noticed when writing to a qcow2-backed image. When virt-v2v is doing a format conversion from raw to qcow2, it does the following: 1. Create a new qcow2 image 2. Launch a libguestfs appliance (kvm) using the new image 3. Write the source raw data to the appliance's block device I noticed that the same process writing to a raw image rather than a qcow2 image was adequately fast, and decided to do some testing. I've attached my simple test program.[1] It does the following: 1. Start an appliance with test.img as a disk. 2. Format test.img with ext2. 3. Create a file /test 4. Write 256M of data to /test in 2M chunks Only step 4 is timed. I ran the program against test.img prepared in 4 different ways: 1. A sparse raw file:15.3 seconds truncate --size 300M test.img 2. A preallocated raw file: 14.8 seconds fallocate -l 300M test.img 3. A sparse qcow2 file: 223.0 seconds qemu-img create -f qcow2 test.img 300M 4. A metadata preallocated qcow2 file: 14.5 seconds qemu-img create -f qcow2 -o preallocated=metadata test.img 300M With the exception of (3), I ran the test 3 times and took the middle time rounded to 1DP. I saw about 5-10% variation. I only ran the test against (3) once. The precise ordering of 1, 2 and 4 is surprising, but given the variation probably not that interesting: they're all about the same. The interesting thing is that the overhead of qcow2 metadata creation during the test seems to account for a 15x performance penalty. I had a cursory look at metadata preallocation, which I hadn't been aware of before today. Creating a qcow2 image of any size with no preallocation results in a 136k file. If you preallocate the metadata, a sparse file is created large enough to accomodate the entire image, with 136k actually used. In the above 300M case this is 204k. On a slightly more practical 20G image, 3.3M is preallocated. It's also worth noting that the image takes considerably longer to create. On my laptop, creation without preallocation is 'instantaneous' at any size. With preallocation, a 20G image takes 6 seconds to create, and a 100G image takes 26 seconds. libvirt's qemu driver doesn't currently preallocate qcow2 metadata when creating a new image. Given the tiny disk space overhead of the metadata (0.02%) and the small processing overhead of pre-creation relative to subsequent creation on-the-fly, I suggest that the libvirt qemu driver is updated to pre-allocate metadata by default. Thoughts? Matt [1] Note that I'm running this against libguestfs from git, which uses virtio-serial rather than usermode networking for appliance-host communication. This change alone improved the performance of this test by about 10x. If your numbers don't match mine, that's probably why. I don't know off the top of my head if this change has made it into F14 yet. It's definitely not in F13. -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490 write-test.pl Description: Perl program -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] bye to close(), welcome to VIR_(FORCE_)CLOSE()
On Mon, 2010-10-25 at 13:44 +0100, Daniel P. Berrange wrote:
On Mon, Oct 25, 2010 at 08:13:19AM -0400, Stefan Berger wrote:
Index: libvirt-acl/src/libvirt.c
@@ -1544,14 +1544,11 @@ cleanup:
vethDelete(veths[i]);
VIR_FREE(veths[i]);
}
-if (rc != 0 priv-monitor != -1) {
-close(priv-monitor);
-priv-monitor = -1;
-}
-if (parentTty != -1)
-close(parentTty);
-if (logfd != -1)
-close(logfd);
+if (rc != 0)
+VIR_FORCE_CLOSE(priv-monitor);
+VIR_FORCE_CLOSE(parentTty);
+if (VIR_CLOSE(logfd) 0)
+virReportSystemError(errno, %s, _(could not close logfile));
This is reporting an error without returning an error code, so the
caller will still see success.
This hunk is in lxc_driver.c in the function lxcVmStart(). Now if in the
cleanup the logfile cannot be closed, that doesn't mean that the VM
could not be started. I am not sure how to handle this correctly, but if
we report an error, we'd probably need to terminate the VM as well... so
is a VIR_FORCE_CLOSE() the proper solution here?
@@ -2011,8 +2008,7 @@ lxcReconnectVM(void *payload, const char
@@ -457,11 +458,15 @@ phypUUIDTable_WriteFile(virConnectPtr co
}
}
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
return 0;
Again, reporting an error while returning success.
Yes, here I can do better and do a 'goto err'.
err:
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
return -1;
}
This is likely blowing away a previously reported error.
Ok, so should I change this to VIR_FORCE_CLOSE()?
@@ -764,7 +769,9 @@ phypUUIDTable_Pull(virConnectPtr conn)
}
break;
}
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno, _(Could not close %s\n),
+ local_file);
goto exit;
Reporting error while returning success
Will do a 'goto err' here as well.
@@ -6542,8 +6536,10 @@ static int qemudDomainSaveImageClose(int
{
int ret = 0;
-if (fd != -1)
-close(fd);
+if (VIR_CLOSE(fd) 0) {
+virReportSystemError(errno, %s,
+ _(cannot close file));
+}
Reporting error while returning success
.. and change this here to VIR_FORCE_CLOSE.
Index: libvirt-acl/src/storage/storage_backend_fs.c
@@ -94,7 +95,9 @@ static int nlOpen(void)
static void nlClose(int fd)
{
-close(fd);
+if (VIR_CLOSE(fd) 0)
+virReportSystemError(errno,
+ %s,_(cannot close netlink socket));
}
No return status at all - this function likely shouldn't even
exist. Should be replaced with direct calls to VIR_FORCE_CLOSE
and VIR_CLOSE as appropriate, returning correct error codes
if it wants to handle close failures.
I'll remove this function. It existed due to nlOpen() existing.
@@ -418,17 +419,13 @@ virHookCall(int driver, const char *id,
}
cleanup:
-if (pipefd[0] = 0) {
-if (close(pipefd[0]) 0) {
-virReportSystemError(errno, %s,
- _(unable to close pipe for hook input));
-}
-}
-if (pipefd[1] = 0) {
-if (close(pipefd[1]) 0) {
-virReportSystemError(errno, %s,
- _(unable to close pipe for hook input));
-}
+if (VIR_CLOSE(pipefd[0]) 0) {
+virReportSystemError(errno, %s,
+ _(unable to close pipe for hook input));
+}
+if (VIR_CLOSE(pipefd[1]) 0) {
+virReportSystemError(errno, %s,
+ _(unable to close pipe for hook input));
}
Reporting errors while returning success.
Use VIR_FORCE_CLOSE() here and convert to not report an error?
Stefan
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix detection of drive readonly option
So far, readonly=on option is used when qemu supports -device. However,
there are qemu versions which support readonly option with -drive
although they don't have support for -device.
---
src/qemu/qemu_conf.c | 12 ++-
src/qemu/qemu_conf.h |1 +
tests/qemuhelpdata/kvm-83-rhel56 | 141
tests/qemuhelptest.c | 26
...qemuxml2argv-disk-drive-readonly-no-device.args |1 +
.../qemuxml2argv-disk-drive-readonly-no-device.xml | 31 +
tests/qemuxml2argvtest.c |5 +-
7 files changed, 214 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuhelpdata/kvm-83-rhel56
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-no-device.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-readonly-no-device.xml
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e2c67a3..00e89a1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1187,6 +1187,8 @@ static unsigned long long qemudComputeCmdFlags(const char
*help,
flags |= QEMUD_CMD_FLAG_DRIVE_CACHE_V2;
if (strstr(help, format=))
flags |= QEMUD_CMD_FLAG_DRIVE_FORMAT;
+if (strstr(help, readonly=))
+flags |= QEMUD_CMD_FLAG_DRIVE_READONLY;
}
if (strstr(help, -vga) !strstr(help, -std-vga))
flags |= QEMUD_CMD_FLAG_VGA;
@@ -1202,8 +1204,14 @@ static unsigned long long qemudComputeCmdFlags(const
char *help,
flags |= QEMUD_CMD_FLAG_CHARDEV;
if (strstr(help, -balloon))
flags |= QEMUD_CMD_FLAG_BALLOON;
-if (strstr(help, -device))
+if (strstr(help, -device)) {
flags |= QEMUD_CMD_FLAG_DEVICE;
+/*
+ * When -device was introduced, qemu already supported drive's
+ * readonly option but didn't advertise that.
+ */
+flags |= QEMUD_CMD_FLAG_DRIVE_READONLY;
+}
if (strstr(help, -nodefconfig))
flags |= QEMUD_CMD_FLAG_NODEFCONFIG;
/* The trailing ' ' is important to avoid a bogus match */
@@ -2688,7 +2696,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
disk-bus != VIR_DOMAIN_DISK_BUS_IDE)
virBufferAddLit(opt, ,boot=on);
if (disk-readonly
-qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)
+qemuCmdFlags QEMUD_CMD_FLAG_DRIVE_READONLY)
virBufferAddLit(opt, ,readonly=on);
if (disk-driverType *disk-driverType != '\0'
disk-type != VIR_DOMAIN_DISK_TYPE_DIR
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 005031d..530dcdb 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -96,6 +96,7 @@ enum qemud_cmd_flags {
QEMUD_CMD_FLAG_FSDEV = (1LL 40), /* -fstype filesystem
passthrough */
QEMUD_CMD_FLAG_NESTING = (1LL 41), /* -enable-nesting (SVM/VMX) */
QEMUD_CMD_FLAG_NAME_PROCESS = (1LL 42), /* Is -name process= available
*/
+QEMUD_CMD_FLAG_DRIVE_READONLY= (1LL 43), /* -drive readonly=on|off
*/
};
/* Main driver state */
diff --git a/tests/qemuhelpdata/kvm-83-rhel56 b/tests/qemuhelpdata/kvm-83-rhel56
new file mode 100644
index 000..b2cefa9
--- /dev/null
+++ b/tests/qemuhelpdata/kvm-83-rhel56
@@ -0,0 +1,141 @@
+QEMU PC emulator version 0.9.1 (kvm-83-maint-snapshot-20090205), Copyright (c)
2003-2008 Fabrice Bellard
+usage: qemu [options] [disk_image]
+
+'disk_image' is a raw hard image image for IDE hard disk 0
+
+Standard options:
+-M machine select emulated machine (-M ? for list)
+-cpu cpuselect CPU (-cpu ? for list)
+-fda/-fdb file use 'file' as floppy disk 0/1 image
+-hda/-hdb file use 'file' as IDE hard disk 0/1 image
+-hdc/-hdd file use 'file' as IDE hard disk 2/3 image
+-cdrom file use 'file' as IDE cdrom image (cdrom is ide1 master)
+-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]
+ [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]
+ [,cache=writethrough|writeback|none|unsafe][,format=f][,serial=s]
+ [,boot=on|off][,readonly=on|off]
+use 'file' as a drive image
+-mtdblock file use 'file' as on-board Flash memory image
+-sd fileuse 'file' as SecureDigital card image
+-pflash fileuse 'file' as a parallel flash image
+-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)
+-snapshot write to temporary files instead of disk image files
+-no-frame open SDL window without a frame and window decorations
+-alt-grab use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)
+-no-quitdisable SDL window close capability
+-no-fd-bootchk disable boot signature checking for floppy disks
+-m megs set virtual RAM size to megs MB [default=128]
+-smp n set the number of CPUs to 'n' [default=1]
+-nographic disable graphical output and redirect serial I/Os to console
+-portrait rotate graphical output 90 deg
Re: [libvirt] Changing the default for qcow2 creation
On Mon, Oct 25, 2010 at 02:40:41PM +0100, Matthew Booth wrote: I've recently been investigating a severe performance issue I noticed when writing to a qcow2-backed image. When virt-v2v is doing a format conversion from raw to qcow2, it does the following: 1. Create a new qcow2 image 2. Launch a libguestfs appliance (kvm) using the new image 3. Write the source raw data to the appliance's block device I noticed that the same process writing to a raw image rather than a qcow2 image was adequately fast, and decided to do some testing. I've attached my simple test program.[1] It does the following: 1. Start an appliance with test.img as a disk. 2. Format test.img with ext2. 3. Create a file /test 4. Write 256M of data to /test in 2M chunks Only step 4 is timed. I ran the program against test.img prepared in 4 different ways: 1. A sparse raw file:15.3 seconds truncate --size 300M test.img 2. A preallocated raw file: 14.8 seconds fallocate -l 300M test.img 3. A sparse qcow2 file: 223.0 seconds qemu-img create -f qcow2 test.img 300M 4. A metadata preallocated qcow2 file: 14.5 seconds qemu-img create -f qcow2 -o preallocated=metadata test.img 300M With the exception of (3), I ran the test 3 times and took the middle time rounded to 1DP. I saw about 5-10% variation. I only ran the test against (3) once. The precise ordering of 1, 2 and 4 is surprising, but given the variation probably not that interesting: they're all about the same. The interesting thing is that the overhead of qcow2 metadata creation during the test seems to account for a 15x performance penalty. I had a cursory look at metadata preallocation, which I hadn't been aware of before today. Creating a qcow2 image of any size with no preallocation results in a 136k file. If you preallocate the metadata, a sparse file is created large enough to accomodate the entire image, with 136k actually used. In the above 300M case this is 204k. On a slightly more practical 20G image, 3.3M is preallocated. It's also worth noting that the image takes considerably longer to create. On my laptop, creation without preallocation is 'instantaneous' at any size. With preallocation, a 20G image takes 6 seconds to create, and a 100G image takes 26 seconds. libvirt's qemu driver doesn't currently preallocate qcow2 metadata when creating a new image. Given the tiny disk space overhead of the metadata (0.02%) and the small processing overhead of pre-creation relative to subsequent creation on-the-fly, I suggest that the libvirt qemu driver is updated to pre-allocate metadata by default. Thoughts? Your test might run faster if you did: $g-copy_size (/dev/zero, /test, size of file); However I think making the change is a no-brainer. We should add this flag by default. [1] Note that I'm running this against libguestfs from git, which uses virtio-serial rather than usermode networking for appliance-host communication. This change alone improved the performance of this test by about 10x. If your numbers don't match mine, that's probably why. I don't know off the top of my head if this change has made it into F14 yet. It's definitely not in F13. F13 and F14 both use virtio-serial now. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix formatting of network address in iptables helpers
The network address was being set to 192.168.122.0 instead
of 192.168.122.0/24. Fix this by removing the unneccessary
'network' field from virNetworkDef and just pass the
network address and netmask into the iptables APIs directly.
* src/conf/network_conf.h, src/conf/network_conf.c: Remove
the 'network' field from virNEtworkDef.
* src/network/bridge_driver.c: Update for iptables API changes
* src/util/iptables.c, src/util/iptables.h: Require the
network address + netmask pair to be passed in
---
src/conf/network_conf.c |4 -
src/conf/network_conf.h |1 -
src/network/bridge_driver.c | 62 --
src/util/iptables.c | 149 +--
src/util/iptables.h | 24 +--
5 files changed, 142 insertions(+), 98 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0bc5a54..3f2bf1f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -438,10 +438,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
-def-network = def-ipAddress;
-def-network.data.inet4.sin_addr.s_addr =
-def-netmask.data.inet4.sin_addr.s_addr;
-
if ((ip = virXPathNode(./ip[1], ctxt))
virNetworkIPParseXML(def, ip) 0)
goto error;
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 5a01bbf..7d31693 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -72,7 +72,6 @@ struct _virNetworkDef {
virSocketAddr ipAddress;/* Bridge IP address */
virSocketAddr netmask;
-virSocketAddr network;
unsigned int nranges;/* Zero or more dhcp ranges */
virNetworkDHCPRangeDefPtr ranges;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5ee47eb..8721747 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -671,7 +671,8 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
int err;
/* allow forwarding packets from the bridge interface */
if ((err = iptablesAddForwardAllowOut(driver-iptables,
- network-def-network,
+ network-def-ipAddress,
+ network-def-netmask,
network-def-bridge,
network-def-forwardDev))) {
virReportSystemError(err,
@@ -682,9 +683,10 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
/* allow forwarding packets to the bridge interface if they are part of an
existing connection */
if ((err = iptablesAddForwardAllowRelatedIn(driver-iptables,
- network-def-network,
- network-def-bridge,
- network-def-forwardDev))) {
+network-def-ipAddress,
+network-def-netmask,
+network-def-bridge,
+network-def-forwardDev))) {
virReportSystemError(err,
_(failed to add iptables rule to allow
forwarding to '%s'),
network-def-bridge);
@@ -716,7 +718,8 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
/* First the generic masquerade rule for other protocols */
if ((err = iptablesAddForwardMasquerade(driver-iptables,
-network-def-network,
+network-def-ipAddress,
+network-def-netmask,
network-def-forwardDev,
NULL))) {
virReportSystemError(err,
@@ -727,7 +730,8 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
/* UDP with a source port restriction */
if ((err = iptablesAddForwardMasquerade(driver-iptables,
-network-def-network,
+network-def-ipAddress,
+network-def-netmask,
network-def-forwardDev,
udp))) {
virReportSystemError(err,
@@ -738,7 +742,8 @@ networkAddMasqueradingIptablesRules(struct network_driver
*driver,
/* TCP with a source port restriction */
if ((err = iptablesAddForwardMasquerade(driver-iptables,
-network-def-network,
+network-def-ipAddress,
+network-def-netmask,
Re: [libvirt] Changing the default for qcow2 creation
On 10/26/2010 01:06 AM, Richard W.M. Jones wrote: snip 4. A metadata preallocated qcow2 file:14.5 seconds qemu-img create -f qcow2 -o preallocated=metadata test.img 300M Just tested this on a F13 host, and it didn't like the the preallocated option: $ qemu-img create -f qcow2 -o preallocated=metadata testvm.qcow2 20G Unknown option 'preallocated' qemu-img: Invalid options for file format 'qcow2'. $ Looks like we'd need to add some kind of testing for the capability first. :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] [TCK] network: create networks and check for exected results
On Fri, Oct 22, 2010 at 08:06:36PM -0400, Stefan Berger wrote: Derived from the nwfilter test program, this one works with libvirt's networks. It creates networks from provided xml files and checks for expected configuration in iptables, running processes and virsh output using provided data files with commands to execute and the expected results for after creating the network and after tearing it down (*.post.dat). 3 tests are currently not passing due to a bug in libvirt... I've just posted a patch that should hopefully fix the problems seen here Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] visrh dump compression support
On Mon, Oct 25, 2010 at 09:04:10AM +0900, KAMEZAWA Hiroyuki wrote:
Sorry, email was empty..
==
Add dump_image_format[] to qemu.conf and support compressed dump
at virsh dump. coredump compression is important for saving disk space
in an environment where multiple guest run.
(In general, disk space for dump is specially allocated and will be
a dead space in the system. It's used only at emergency. So, it's better
to have both of save_image_format and dump_image_format. save is done
in scheduled manner with enough calculated disk space for it.)
This code reuses some of save_image_format[] and supports the same format
with virsh save.
---
src/qemu/qemu.conf |4
src/qemu/qemu_conf.c | 11 +++
src/qemu/qemu_conf.h |1 +
src/qemu/qemu_driver.c | 30 +-
4 files changed, 41 insertions(+), 5 deletions(-)
This all looks good, but it is also neccessary to add the new
option to the augeas files libvirtd_qemu.aug and
test_libvirtd_qemu.aug
Index: libvirt-0.8.4/src/qemu/qemu_conf.c
===
--- libvirt-0.8.4.orig/src/qemu/qemu_conf.c
+++ libvirt-0.8.4/src/qemu/qemu_conf.c
@@ -324,6 +324,17 @@ int qemudLoadDriverConfig(struct qemud_d
}
}
+p = virConfGetValue (conf, dump_image_format);
+CHECK_TYPE (dump_image_format, VIR_CONF_STRING);
+if (p p-str) {
+ VIR_FREE(driver-dumpImageFormat);
+ if (!(driver-dumpImageFormat = strdup(p-str))) {
+ virReportOOMError();
+ virConfFree(conf);
+ return -1;
+ }
+}
+
p = virConfGetValue (conf, hugetlbfs_mount);
CHECK_TYPE (hugetlbfs_mount, VIR_CONF_STRING);
if (p p-str) {
Index: libvirt-0.8.4/src/qemu/qemu_conf.h
===
--- libvirt-0.8.4.orig/src/qemu/qemu_conf.h
+++ libvirt-0.8.4/src/qemu/qemu_conf.h
@@ -159,6 +159,7 @@ struct qemud_driver {
virSecurityDriverPtr securitySecondaryDriver;
char *saveImageFormat;
+char *dumpImageFormat;
pciDeviceList *activePciHostdevs;
Index: libvirt-0.8.4/src/qemu/qemu.conf
===
--- libvirt-0.8.4.orig/src/qemu/qemu.conf
+++ libvirt-0.8.4/src/qemu/qemu.conf
@@ -144,7 +144,11 @@
# saving a domain in order to save disk space; the list above is in
descending
# order by performance and ascending order by compression ratio.
#
+# save_image_format is used when you use 'virsh save' at scheduled saving.
+# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
+#
# save_image_format = raw
+# dump_image_format = raw
# If provided by the host and a hugetlbfs mount point is configured,
# a guest may request huge page backing. When this mount point is
Index: libvirt-0.8.4/src/qemu/qemu_driver.c
===
--- libvirt-0.8.4.orig/src/qemu/qemu_driver.c
+++ libvirt-0.8.4/src/qemu/qemu_driver.c
@@ -5716,11 +5716,15 @@ static int qemudDomainCoreDump(virDomain
int resume = 0, paused = 0;
int ret = -1, fd = -1;
virDomainEventPtr event = NULL;
-const char *args[] = {
-cat,
-NULL,
-};
+int compress;
qemuDomainObjPrivatePtr priv;
+/*
+ * We reuse save flag for dump here. Then, we can support the same
+ * format in save and dump.
+ */
+compress = QEMUD_SAVE_FORMAT_RAW;
+if (driver-dumpImageFormat)
+ compress = qemudSaveCompressionTypeFromString(driver-dumpImageFormat);
qemuDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
@@ -5787,9 +5791,25 @@ static int qemudDomainCoreDump(virDomain
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-ret = qemuMonitorMigrateToFile(priv-mon,
+if (compress == QEMUD_SAVE_FORMAT_RAW) {
+ const char *args[] = {
+ cat,
+ NULL,
+ };
+ ret = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path, 0);
+} else {
+ const char *prog = qemudSaveCompressionTypeToString(compress);
+ const char *args[] = {
+ prog,
+ -c,
+ NULL,
+ };
+ ret = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
+}
The whitespace indentation looks wrong here, seems to be using
tabs instead of spaces. You can verify coding style by running
'make syntax-check'
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o-
Re: [libvirt] [PATCH v2 2/2] confirm compression program availability
On Mon, Oct 25, 2010 at 09:05:29AM +0900, KAMEZAWA Hiroyuki wrote:
At compression, external programs are used but it is not checked whether
the program is available or not.
Check it at parsing qemu.conf and find problems in early stage.
The problem with doing the error check here is that it will
cause the entire libvirtd to fail to startup, and just syslog
the error. We've found admins often miss these types of problem
in syslog and just file bugs complaining that libvirtd doesn't
start.
I think we should put an explicit check for existance of the
compression program in the API code instead. That way the
error message will get fed to the end user who will easily
notice it.
---
src/qemu/qemu_conf.c | 46 +++---
1 file changed, 39 insertions(+), 7 deletions(-)
Index: libvirt-0.8.4/src/qemu/qemu_conf.c
===
--- libvirt-0.8.4.orig/src/qemu/qemu_conf.c
+++ libvirt-0.8.4/src/qemu/qemu_conf.c
@@ -316,22 +316,54 @@ int qemudLoadDriverConfig(struct qemud_d
p = virConfGetValue (conf, save_image_format);
CHECK_TYPE (save_image_format, VIR_CONF_STRING);
if (p p-str) {
-VIR_FREE(driver-saveImageFormat);
-if (!(driver-saveImageFormat = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-return -1;
-}
+ int find = 1;
+ if (strcmp(p-str, raw)) {
+ char *c;
+ c = virFindFileInPath(p-str);
+ if (!c)
+ find = 0;
+ else
+ VIR_FREE(c);
+ }
+ VIR_FREE(driver-saveImageFormat);
+ if (find) {
+if (!(driver-saveImageFormat = strdup(p-str))) {
+ virReportOOMError();
+ virConfFree(conf);
+ return -1;
+ }
+ } else {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+save_image_format cannot find program %s, p-str);
+ virConfFree(conf);
+ return -1;
+ }
}
p = virConfGetValue (conf, dump_image_format);
CHECK_TYPE (dump_image_format, VIR_CONF_STRING);
if (p p-str) {
+ int find = 1;
+ if (strcmp(p-str, raw)) {
+char *c;
+ c = virFindFileInPath(p-str);
+ if (!c)
+ find = 0;
+ else
+ VIR_FREE(c);
+ }
VIR_FREE(driver-dumpImageFormat);
- if (!(driver-dumpImageFormat = strdup(p-str))) {
+ if (find) {
+ if (!(driver-dumpImageFormat = strdup(p-str))) {
virReportOOMError();
virConfFree(conf);
return -1;
+ }
+ } else {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+dump_image_format cannot find program %s, p-str);
+ virConfFree(conf);
+ return -1;
}
}
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] qemu: call drive_unplug in DetachPciDiskDevice
On Fri, Oct 22, 2010 at 09:14:22AM -0500, Ryan Harper wrote:
Currently libvirt doesn't confirm whether the guest has responded to the
disk removal request. In some cases this can leave the guest with
continued access to the device while the mgmt layer believes that it has
been removed. With a recent qemu monitor command[1] we can
deterministically revoke a guests access to the disk (on the QEMU side)
to ensure no futher access is permitted.
This patch adds support for the drive_unplug() command and introduces it
in the disk removal paths. There is some discussion to be had about how
to handle the case where the guest is running in a QEMU without this
command (and the fact that we currently don't have a way of detecting
what monitor commands are available).
Changes since v2:
- use VIR_ERROR to report when unplug command not found
Changes since v1:
- return 0 when command isn't present, 0 on command failure
- detect when drive_unplug command isn't present and log error
instead of failing entire command
Signed-off-by: Ryan Harper ry...@us.ibm.com
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index abd8e9d..615427a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8646,6 +8646,7 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
virCgroupPtr cgroup = NULL;
+char drivestr[PATH_MAX];
i = qemudFindDisk(vm-def, dev-data.disk-dst);
@@ -8673,13 +8674,36 @@ static int qemudDomainDetachPciDiskDevice(struct
qemud_driver *driver,
goto cleanup;
}
+/* build the actual drive id string as the disk-info.alias doesn't
+ * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */
+if ((ret = snprintf(drivestr, sizeof(drivestr), %s%s,
+ QEMU_DRIVE_HOST_PREFIX,
+ detach-info.alias))
+ 0 || ret = sizeof(drivestr)) {
+virReportOOMError();
+goto cleanup;
+}
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) {
+ret = qemuMonitorDriveUnplug(priv-mon, drivestr);
+DEBUG(DriveUnplug ret=%d, ret);
+/* ret 0 indicates unplug isn't supported, issue will be logged */
+if (ret 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
if (qemuMonitorDelDevice(priv-mon, detach-info.alias) 0) {
qemuDomainObjExitMonitor(vm);
goto cleanup;
}
} else {
+ret = qemuMonitorDriveUnplug(priv-mon, drivestr);
+/* ret 0 indicates unplug isn't supported, issue will be logged */
+if (ret 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
if (qemuMonitorRemovePCIDevice(priv-mon,
detach-info.addr.pci) 0) {
qemuDomainObjExitMonitor(vm);
@@ -8723,6 +8747,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct
qemud_driver *driver,
virDomainDiskDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm-privateData;
virCgroupPtr cgroup = NULL;
+char drivestr[PATH_MAX];
i = qemudFindDisk(vm-def, dev-data.disk-dst);
@@ -8749,7 +8774,22 @@ static int qemudDomainDetachSCSIDiskDevice(struct
qemud_driver *driver,
}
}
+/* build the actual drive id string as the disk-info.alias doesn't
+ * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */
+if ((ret = snprintf(drivestr, sizeof(drivestr), %s%s,
+ QEMU_DRIVE_HOST_PREFIX,
+ detach-info.alias))
+ 0 || ret = sizeof(drivestr)) {
+virReportOOMError();
+goto cleanup;
+}
+
qemuDomainObjEnterMonitorWithDriver(driver, vm);
+/* ret 0 indicates unplug isn't supported, issue will be logged */
+if (qemuMonitorDriveUnplug(priv-mon, drivestr) 0) {
+qemuDomainObjExitMonitor(vm);
+goto cleanup;
+}
if (qemuMonitorDelDevice(priv-mon, detach-info.alias) 0) {
qemuDomainObjExitMonitor(vm);
goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2366fdb..285381d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1781,6 +1781,25 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon,
return ret;
}
+int qemuMonitorDriveUnplug(qemuMonitorPtr mon,
+ const char *drivestr)
+{
+DEBUG(mon=%p drivestr=%s, mon, drivestr);
+int ret;
+
+if (!mon) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(monitor must not be NULL));
+return -1;
+}
+
+if (mon-json)
+ret = qemuMonitorJSONDriveUnplug(mon, drivestr);
+else
+ret =
Re: [libvirt] Changing the default for qcow2 creation
On Tue, Oct 26, 2010 at 02:11:22AM +1100, Justin Clift wrote: On 10/26/2010 01:06 AM, Richard W.M. Jones wrote: snip 4. A metadata preallocated qcow2 file: 14.5 seconds qemu-img create -f qcow2 -o preallocated=metadata test.img 300M Just tested this on a F13 host, and it didn't like the the preallocated option: $ qemu-img create -f qcow2 -o preallocated=metadata testvm.qcow2 20G Unknown option 'preallocated' qemu-img: Invalid options for file format 'qcow2'. $ Looks like we'd need to add some kind of testing for the capability first. :) The option should be '-o preallocation=metadata'. This works for me on Fedora 13 too. This option was introduced to qemu upstream in commit a35e1c177debb01240243bd656caca302410d38c (Aug 17 2009). You could actually detect this at runtime by running: $ qemu-img create -f qcow2 -o preallocation=metadata /dev/null 1M Formatting '/dev/null', fmt=qcow2 size=1048576 encryption=off cluster_size=0 preallocation='metadata' $ echo $? 0 $ qemu-img create -f qcow2 -o preallocation=foobar /dev/null 1M Formatting '/dev/null', fmt=qcow2 size=1048576 encryption=off cluster_size=0 preallocation='foobar' Invalid preallocation mode: 'foobar' qemu-img: Error while formatting $ echo $? 1 However the first command segfaults on Fedora 14 (so near and yet so far!) I'm going to file a bug about that. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] [TCK] network: create networks and check for expected results
V2:
- test cases for ipconfig, brctl added
- fixed test data after after using iptables with -n
Derived from the nwfilter test program, this one works with libvirt's
networks. It creates networks from provided xml files and checks for
expected configuration in iptables, running processes and virsh output
using provided data files with commands to execute and the expected
results for after creating the network and after tearing it down
(*.post.dat). 3 tests are currently not passing due to a bug in
libvirt...
Signed-off-by: Stefan Bergerstef...@us.ibm.com
---
Build.PL |2
scripts/networks/100-apply-verify-host.t | 10
scripts/networks/networkApplyTest.sh | 343 +
scripts/networks/networkxml2hostout/tck-testnet-1.dat | 17
scripts/networks/networkxml2hostout/tck-testnet-1.post.dat |7
scripts/networks/networkxml2hostout/tck-testnet-2.dat | 14
scripts/networks/networkxml2hostout/tck-testnet-2.post.dat |7
scripts/networks/networkxml2xmlin/tck-testnet-1.xml| 12
scripts/networks/networkxml2xmlin/tck-testnet-2.xml| 12
9 files changed, 423 insertions(+), 1 deletion(-)
Index: libvirt-tck/scripts/networks/networkApplyTest.sh
===
--- /dev/null
+++ libvirt-tck/scripts/networks/networkApplyTest.sh
@@ -0,0 +1,343 @@
+#!/bin/bash
+
+VIRSH=virsh
+
+uri=
+if [ x${LIBVIRT_TCK_CONFIG}x != xx ]; then
+uri_exp=`cat ${LIBVIRT_TCK_CONFIG} | grep ^uri\s*= | sed -e
's/uri\s*=\s*//' | tail -n 1`
+if [ x${uri_exp}x != xx ]; then
+eval uri=${uri_exp}
+fi
+if [ x${uri}x == xx ]; then
+uri=qemu:///system
+fi
+else
+uri=qemu:///system
+fi
+LIBVIRT_URI=${uri}
+
+
+FLAG_WAIT=$((10))
+FLAG_VERBOSE=$((12))
+FLAG_LIBVIRT_TEST=$((13))
+FLAG_TAP_TEST=$((14))
+FLAG_FORCE_CLEAN=$((15))
+
+failctr=0
+passctr=0
+attachfailctr=0
+attachctr=0
+
+TAP_FAIL_LIST=
+TAP_FAIL_CTR=0
+TAP_TOT_CTR=0
+
+function usage() {
+ local cmd=$0
+catEOF
+Usage: ${cmd} [--help|-h|-?] [--noattach] [--wait] [--verbose]
+ [--libvirt-test] [--tap-test]
+
+Options:
+ --help,-h,-? : Display this help screen.
+ --wait : Wait for the user to press the enter key once an error
+ was detected
+ --verbose : Verbose output
+ --libvirt-test : Use the libvirt test output format
+ --tap-test : TAP format output
+ --force: Allow the automatic cleaning of VMs and networks
+ previously created by the TCK test suite
+
+This test creates libvirt 'networks' and checks for expected results
+(iptables, running processes (dnsmasq)) using provided xml and data
+file respectively.
+EOF
+}
+
+
+function tap_fail() {
+ echo not ok $1 - ${2:0:66}
+ TAP_FAIL_LIST+=$1
+ ((TAP_FAIL_CTR++))
+ ((TAP_TOT_CTR++))
+}
+
+function tap_pass() {
+ echo ok $1 - ${2:0:70}
+ ((TAP_TOT_CTR++))
+}
+
+function tap_final() {
+ local okay
+
+ [ -n ${TAP_FAIL_LIST} ] echo FAILED tests ${TAP_FAIL_LIST}
+
+ okay=`echo ($TAP_TOT_CTR-$TAP_FAIL_CTR)*100/$TAP_TOT_CTR | bc -l`
+ echo Failed ${TAP_FAIL_CTR}/${TAP_TOT_CTR} tests, ${okay:0:5}% okay
+}
+
+# A wrapper for mktemp in case it does not exist
+# Echos the name of a temporary file.
+function mktmpfile() {
+ local tmp
+ type -P mktemp /dev/null
+ if [ $? -eq 0 ]; then
+tmp=$(mktemp -t nwfvmtest.XX)
+echo ${tmp}
+ else
+while :; do
+ tmp=/tmp/nwfvmtest.${RANDOM}
+ if [ ! -f ${tmp} ]; then
+ touch ${tmp}
+ chmod 666 ${tmp}
+ echo ${tmp}
+ break
+ fi
+done
+ fi
+ return 0
+}
+
+
+function checkExpectedOutput() {
+ local xmlfile=$1
+ local datafile=$2
+ local flags=$3
+ local skipregex=$4
+ local cmd line tmpfile tmpfile2 skip
+
+ tmpfile=`mktmpfile`
+ tmpfile2=`mktmpfile`
+
+ exec 4${datafile}
+
+ read4
+ line=${REPLY}
+
+ while [ x${line}x != xx ]; do
+cmd=`echo ${line##\#}`
+
+skip=0
+if [ x${skipregex}x != xx ]; then
+ skip=`echo ${cmd} | grep -c -E ${skipregex}`
+fi
+
+eval ${cmd} 21 | tee ${tmpfile} 1/dev/null
+
+rm ${tmpfile2} 2/dev/null
+touch ${tmpfile2}
+
+while [ 1 ]; do
+ read4
+ line=${REPLY}
+
+ if [ ${line:0:1} == # ] || [ x${line}x == xx ]; then
+
+ if [ ${skip} -ne 0 ]; then
+ break
+ fi
+
+diff ${tmpfile} ${tmpfile2}/dev/null
+
+if [ $? -ne 0 ]; then
+ if [ $((flags FLAG_VERBOSE)) -ne 0 ]; then
+echo FAIL ${xmlfile} : ${cmd}
+diff ${tmpfile} ${tmpfile2}
+ fi
+ ((failctr++))
+ if [ $((flags FLAG_WAIT)) -ne 0 ]; then
+echo tmp files: $tmpfile, $tmpfile2
+ echo Press enter
+ read
+ fi
+ [ $((flags FLAG_LIBVIRT_TEST)) -ne 0 ] \
+ test_result $((passctr+failctr)) 1
+ [ $((flags
Re: [libvirt] [PATCH] Fix formatting of network address in iptables helpers
On 10/25/2010 11:04 AM, Daniel P. Berrange wrote: The network address was being set to 192.168.122.0 instead of 192.168.122.0/24. Fix this by removing the unneccessary 'network' field from virNetworkDef and just pass the network address and netmask into the iptables APIs directly. * src/conf/network_conf.h, src/conf/network_conf.c: Remove the 'network' field from virNEtworkDef. * src/network/bridge_driver.c: Update for iptables API changes * src/util/iptables.c, src/util/iptables.h: Require the network address + netmask pair to be passed in --- src/conf/network_conf.c |4 - src/conf/network_conf.h |1 - src/network/bridge_driver.c | 62 -- src/util/iptables.c | 149 +-- src/util/iptables.h | 24 +-- 5 files changed, 142 insertions(+), 98 deletions(-) [ [...] int iptablesRemoveForwardMasquerade (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, Locally applied and passes v2 of the test. ACK. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] List of available libvirt drivers installed?
I am wondering if there is a way to show a list of available libvirt hypervisor drvers installed in an existing libvirt installation. I know based on the version of libvirt, I can find out supported drivers from this matrix: http://libvirt.org/hvsupport.html However, the particular installation may not turn on all available drivers for this particular build. Is there a way to find out? Particularly, I want to find out if the default libvirt coming with RHEL-5/6 supports VMware ESX, and the same for Ubuntu-10.4/10.10. Thanks in advance. Shi -- Shi Jin, PhD -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ping on pending patches
I have several pending patches that are probably worth inclusion in 0.8.5; would anyone like to review them? https://www.redhat.com/archives/libvir-list/2010-October/msg00726.html - virsh help text relating to integer arguments https://www.redhat.com/archives/libvir-list/2010-October/msg00728.html - virsh memtune memory leak and arbitrary limitation fix https://www.redhat.com/archives/libvir-list/2010-October/msg00895.html - work around dash 0.5.5 bug affecting managed save https://www.redhat.com/archives/libvir-list/2010-October/msg00711.html - api_extension docs -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Empty Domain Management docs page?
On 10/23/2010 01:59 PM, Justin Clift wrote: Hi all, At the moment, the Domain Management page in the docs is empty: http://libvirt.org/archdomain.html Guessing it's as some kind of placeholder. Ideally, what should the contents of this page be? Maybe an overview of a rough life cycle of a domain, contrasting transient and persistent domains. I've seen a state diagram of this before, but not sure where. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: reformated the bindings page html markup to match other pages
On 10/23/2010 03:09 PM, Justin Clift wrote: --- Looks the same when processed, but the previous messy HTML got on my nerves while I was looking at other stuff. docs/bindings.html.in | 74 +--- 1 files changed, 51 insertions(+), 23 deletions(-) ACK; and since there's no content change, you could probably push future changes like this under the trivial rule (but do continue to send even trivial patches to the list). +strongPerl/strong: Daniel Berrange provides +a href=http://search.cpan.org/dist/Sys-Virt/;bindings for Perl/a. +/li +li +strongOCaml/strong: Richard Jones supplies +a href=http://libvirt.org/ocaml/;bindings for OCaml/a. +/li +li +strongRuby/strong: David Lutterkort provides +a href=http://libvirt.org/ruby/;bindings for Ruby/a. +/li +li +strongJava/strong: Daniel Veillard maintains Provides, supplies, maintains, developing - can we come up with any more synonyms? Of course, not an issue for this whitespace-only patch, but I might as well mention it while noticing it. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: added a table of contents to the first 11 docs files
On 10/23/2010 03:54 PM, Justin Clift wrote: There are a 58 docs files, so adding an autogeneratec Table Of Contents to them all will take some time. This is the first piece of the work done. --- docs/api.html.in | 17 +++-- docs/apps.html.in | 29 ++--- docs/architecture.html.in | 42 +++--- docs/archnetwork.html.in |9 ++--- docs/auth.html.in | 41 ++--- docs/bugs.html.in | 12 docs/contact.html.in |6 -- docs/deployment.html.in | 10 ++ docs/devguide.html.in |6 -- docs/downloads.html.in| 18 ++ docs/drivers.html.in | 28 ++-- 11 files changed, 114 insertions(+), 104 deletions(-) ACK. Do the makefile rules already take care of generating the toc, or is that a later patch? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: make the location of the xml catalog file a configure option
On 10/24/2010 01:39 AM, Justin Clift wrote: The default location for the XML catalog file, /etc/xml/catalog, used when validating the generated html docs, isn't correct for MacOS X. This commit adds an option to the configure script, allowing the default to be overridden: --with-xml-catalog-file=/path/to/xml/catalog/file --- Tested on MacOS X 10.6 (using overridden locatin), and Fedora 13 (using the default location). Works fine on both. ACK with two nits: +dnl Specific XML catalog file for validation of generated html +AC_ARG_WITH([xml-catalog-file], [AC_HELP_STRING([--with-xml-catalog-file=path], +[path to XML catalog file for validating generated html, default /etc/xml/catalog])], this line is long; you can put arbitrary line breaks in the second argument to AC_HELP_STRING, and autoconf will automatically re-fill the line to 80 columns in the ./configure --help output. So, to avoid a long line, just do: AC_ARG_WITH([xml-catalog-file], [AC_HELP_STRING([--with-xml-catalog-file=path], [path to XML catalog file for validating generated html, default /etc/xml/catalog])], [XML_CATALOG_FILE=$withval], [XML_CATALOG_FILE='/etc/xml/catalog']) %.html: %.html.tmp @if test -x $(XMLLINT) test -x $(XMLCATALOG) ; then \ - if $(XMLCATALOG) /etc/xml/catalog \ + if $(XMLCATALOG) $(XML_CATALOG_FILE) \ Should we worry about potential whitespace in $(XML_CATALOG_FILE)? That is, except for single quote, we can make this more robust by using: if $(XMLCATALOG) '$(XML_CATALOG_FILE)' and so forth for all Makefile references to this variable. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] docs: install the generated html files when make install is run
On 10/24/2010 03:34 AM, Justin Clift wrote: Previously, only the API docs were installed, rather than the complete documentation set. This commit ensures the complete documentation set is installed. --- docs/Makefile.am | 11 ++- 1 files changed, 10 insertions(+), 1 deletions(-) @@ -169,6 +174,10 @@ rebuild: api all install-data-local: $(mkinstalldirs) $(DESTDIR)$(HTML_DIR) + for f in $(css) $(dot_html); do \ + $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR); done + for f in $(gif) $(png); do \ + $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR); done Any reason this is done as two for loops instead of one? ACK once you rewrite this as: for f in $(css) $(dot_html) $(gif) $(png); do \ $(INSTALL) -m 0644 $(srcdir)/$$f $(DESTDIR)$(HTML_DIR); done As a side note, recent automake has gone to efforts to speed up installation by using a single install invocation on multiple target files all going to the same directory. Unfortunately, until we can rely on automake 1.11 being a minimum development requirement, I don't know if we can take advantage of any of those speedups, because older install-sh scripts (necessary on systems where install(1) is less-than-stellar) didn't support multiple arguments. For reference, here's the automake NEWS snippet: - The targets `install' and `uninstall' are more efficient now, in that for example multiple files from one Automake variable such as `bin_SCRIPTS' are copied in one `install' (or `libtool --mode=install') invocation if they do not have to be renamed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] virsh: update attach-interface docs
On 10/25/2010 06:00 AM, Osier Yang wrote: * tools/virsh.pod (attach-interface): add docs for new option 'model' and missed option 'persistent'. --- tools/virsh.pod |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) ACK series. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Initial working Mac OS X libvirt client build
On 10/25/2010 05:19 AM, Ruben Kerkhof wrote: On Mon, Oct 25, 2010 at 10:48, Daniel P. Berrangeberra...@redhat.com wrote: NACK, this results in 'path' being a zer-length string, so no data is copied in the next virStrcpyStatic line. The original code is correctly creating a socket in the abstract namespace, ie one which does not appear in the filesystem Daniel Ah, I didn't knew that, sorry. Am I right in assuming that an abstract namespace is a linux-only feature? The unix manpage on my Mac doesn't describe it, neither does FreeBSD (http://www.freebsd.org/cgi/man.cgi?query=unixapropos=0sektion=0manpath=FreeBSD+8.1-RELEASEformat=html). Correct - sun_path starting with a NUL byte is a Linux extension, and not portable to other hosts (except maybe Cygwin). We need to come up with an alternative to the abstract namespace for use on non-Linux hosts. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] --checksum-fill error on Ubuntu 10.04
Hi us, Just noticed an error message when manually starting libvirtd (built from git) on Ubuntu 10.04, while checking something else: iptables v1.4.4: unknown option `--checksum-fill' It shows up in the initial default network setup: 10:40:19.807: error : virRunWithHook:855 : internal error '/sbin/iptables --table mangle --insert POSTROUTING --out-interface virbr0 --protocol udp --destination-port 68 --jump CHECKSUM --checksum-fill' exited with non-zero status 2 and signal 0: iptables v1.4.4: unknown option `--checksum-fill' Try `iptables -h' or 'iptables --help' for more information. Just a FYI, in case someone wants to look into it. Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] visrh dump compression support
On Mon, 25 Oct 2010 16:18:07 +0100
Daniel P. Berrange berra...@redhat.com wrote:
On Mon, Oct 25, 2010 at 09:04:10AM +0900, KAMEZAWA Hiroyuki wrote:
Sorry, email was empty..
==
Add dump_image_format[] to qemu.conf and support compressed dump
at virsh dump. coredump compression is important for saving disk space
in an environment where multiple guest run.
(In general, disk space for dump is specially allocated and will be
a dead space in the system. It's used only at emergency. So, it's better
to have both of save_image_format and dump_image_format. save is done
in scheduled manner with enough calculated disk space for it.)
This code reuses some of save_image_format[] and supports the same format
with virsh save.
---
src/qemu/qemu.conf |4
src/qemu/qemu_conf.c | 11 +++
src/qemu/qemu_conf.h |1 +
src/qemu/qemu_driver.c | 30 +-
4 files changed, 41 insertions(+), 5 deletions(-)
This all looks good, but it is also neccessary to add the new
option to the augeas files libvirtd_qemu.aug and
test_libvirtd_qemu.aug
Ah, I missed it. I'll look into.
Index: libvirt-0.8.4/src/qemu/qemu_conf.c
===
--- libvirt-0.8.4.orig/src/qemu/qemu_conf.c
+++ libvirt-0.8.4/src/qemu/qemu_conf.c
@@ -324,6 +324,17 @@ int qemudLoadDriverConfig(struct qemud_d
}
}
+p = virConfGetValue (conf, dump_image_format);
+CHECK_TYPE (dump_image_format, VIR_CONF_STRING);
+if (p p-str) {
+ VIR_FREE(driver-dumpImageFormat);
+ if (!(driver-dumpImageFormat = strdup(p-str))) {
+ virReportOOMError();
+ virConfFree(conf);
+ return -1;
+ }
+}
+
p = virConfGetValue (conf, hugetlbfs_mount);
CHECK_TYPE (hugetlbfs_mount, VIR_CONF_STRING);
if (p p-str) {
Index: libvirt-0.8.4/src/qemu/qemu_conf.h
===
--- libvirt-0.8.4.orig/src/qemu/qemu_conf.h
+++ libvirt-0.8.4/src/qemu/qemu_conf.h
@@ -159,6 +159,7 @@ struct qemud_driver {
virSecurityDriverPtr securitySecondaryDriver;
char *saveImageFormat;
+char *dumpImageFormat;
pciDeviceList *activePciHostdevs;
Index: libvirt-0.8.4/src/qemu/qemu.conf
===
--- libvirt-0.8.4.orig/src/qemu/qemu.conf
+++ libvirt-0.8.4/src/qemu/qemu.conf
@@ -144,7 +144,11 @@
# saving a domain in order to save disk space; the list above is in
descending
# order by performance and ascending order by compression ratio.
#
+# save_image_format is used when you use 'virsh save' at scheduled saving.
+# dump_image_format is used when you use 'virsh dump' at emergency
crashdump.
+#
# save_image_format = raw
+# dump_image_format = raw
# If provided by the host and a hugetlbfs mount point is configured,
# a guest may request huge page backing. When this mount point is
Index: libvirt-0.8.4/src/qemu/qemu_driver.c
===
--- libvirt-0.8.4.orig/src/qemu/qemu_driver.c
+++ libvirt-0.8.4/src/qemu/qemu_driver.c
@@ -5716,11 +5716,15 @@ static int qemudDomainCoreDump(virDomain
int resume = 0, paused = 0;
int ret = -1, fd = -1;
virDomainEventPtr event = NULL;
-const char *args[] = {
-cat,
-NULL,
-};
+int compress;
qemuDomainObjPrivatePtr priv;
+/*
+ * We reuse save flag for dump here. Then, we can support the same
+ * format in save and dump.
+ */
+compress = QEMUD_SAVE_FORMAT_RAW;
+if (driver-dumpImageFormat)
+ compress = qemudSaveCompressionTypeFromString(driver-dumpImageFormat);
qemuDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
@@ -5787,9 +5791,25 @@ static int qemudDomainCoreDump(virDomain
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
-ret = qemuMonitorMigrateToFile(priv-mon,
+if (compress == QEMUD_SAVE_FORMAT_RAW) {
+ const char *args[] = {
+ cat,
+ NULL,
+ };
+ ret = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND,
args, path, 0);
+} else {
+ const char *prog = qemudSaveCompressionTypeToString(compress);
+ const char *args[] = {
+ prog,
+ -c,
+ NULL,
+ };
+ ret = qemuMonitorMigrateToFile(priv-mon,
+ QEMU_MONITOR_MIGRATE_BACKGROUND,
+ args, path, 0);
+}
The whitespace indentation looks wrong here, seems to be using
tabs instead of spaces. You can verify coding style by running
'make syntax-check'
Ok. and check my .emacs..
Re: [libvirt] [PATCH v2 2/2] confirm compression program availability
On Mon, 25 Oct 2010 16:20:50 +0100 Daniel P. Berrange berra...@redhat.com wrote: On Mon, Oct 25, 2010 at 09:05:29AM +0900, KAMEZAWA Hiroyuki wrote: At compression, external programs are used but it is not checked whether the program is available or not. Check it at parsing qemu.conf and find problems in early stage. The problem with doing the error check here is that it will cause the entire libvirtd to fail to startup, and just syslog the error. We've found admins often miss these types of problem in syslog and just file bugs complaining that libvirtd doesn't start. Hmm, yes. I think we should put an explicit check for existance of the compression program in the API code instead. That way the error message will get fed to the end user who will easily notice it. Ok, I'll try it. I wonder which is better to make the command fail or run it in raw format when compression program cannot be found. Because virsh dump cannot be stopped by Ctrl-C, starting the dump can be critical decision. I think I should just make it fail and add --raw option to virsh.. Thanks, -Kame -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] virsh: improve help text where integers are expected
On 10/20/2010 02:30 PM, Eric Blake wrote:
* tools/virsh.c (opts_freecell, opts_memtune, opts_vcpupin)
(opts_setvcpus, opts_setmaxmem, opts_setmem)
(opts_migrate_setmaxdowntime): Use VSH_OT_INT when only an integer
is expected.
(vshCmddefHelp, vshCmddefGetData): Allow mandatory VSH_OT_INT
arguments.
---
changes in v2:
Update some infrastructure fo 'make check' passes again.
tools/virsh.c | 37 +
1 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3e37b06..ca9a61e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -119,9 +119,9 @@ typedef enum {
* vshCmdOptType - command option type
*/
typedef enum {
-VSH_OT_BOOL, /* boolean option */
-VSH_OT_STRING, /* string option */
-VSH_OT_INT, /* int option */
+VSH_OT_BOOL, /* optional boolean option */
+VSH_OT_STRING, /* optional string option */
+VSH_OT_INT, /* optional or mandatory int option */
VSH_OT_DATA, /* string data (as non-option) */
VSH_OT_ARGV /* remaining arguments, opt-name should be */
} vshCmdOptType;
@@ -2247,7 +2247,7 @@ static const vshCmdInfo info_freecell[] = {
};
static const vshCmdOptDef opts_freecell[] = {
-{cellno, VSH_OT_DATA, 0, N_(NUMA cell number)},
+{cellno, VSH_OT_INT, 0, N_(NUMA cell number)},
{NULL, 0, 0, NULL}
};
@@ -2583,7 +2583,7 @@ static const vshCmdInfo info_vcpupin[] = {
static const vshCmdOptDef opts_vcpupin[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{vcpu, VSH_OT_DATA, VSH_OFLAG_REQ, N_(vcpu number)},
+{vcpu, VSH_OT_INT, VSH_OFLAG_REQ, N_(vcpu number)},
{cpulist, VSH_OT_DATA, VSH_OFLAG_REQ, N_(host cpu number(s) (comma
separated))},
{NULL, 0, 0, NULL}
};
@@ -2719,7 +2719,7 @@ static const vshCmdInfo info_setvcpus[] = {
static const vshCmdOptDef opts_setvcpus[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{count, VSH_OT_DATA, VSH_OFLAG_REQ, N_(number of virtual CPUs)},
+{count, VSH_OT_INT, VSH_OFLAG_REQ, N_(number of virtual CPUs)},
{maximum, VSH_OT_BOOL, 0, N_(set maximum limit on next boot)},
{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{live, VSH_OT_BOOL, 0, N_(affect running domain)},
@@ -2772,7 +2772,7 @@ static const vshCmdInfo info_setmem[] = {
static const vshCmdOptDef opts_setmem[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{kilobytes, VSH_OT_DATA, VSH_OFLAG_REQ, N_(number of kilobytes of
memory)},
+{kilobytes, VSH_OT_INT, VSH_OFLAG_REQ, N_(number of kilobytes of
memory)},
{NULL, 0, 0, NULL}
};
@@ -2829,7 +2829,7 @@ static const vshCmdInfo info_setmaxmem[] = {
static const vshCmdOptDef opts_setmaxmem[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{kilobytes, VSH_OT_DATA, VSH_OFLAG_REQ, N_(maximum memory limit in
kilobytes)},
+{kilobytes, VSH_OT_INT, VSH_OFLAG_REQ, N_(maximum memory limit in
kilobytes)},
{NULL, 0, 0, NULL}
};
@@ -2890,13 +2890,13 @@ static const vshCmdInfo info_memtune[] = {
static const vshCmdOptDef opts_memtune[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE,
+{VIR_DOMAIN_MEMORY_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE,
N_(Max memory in kilobytes)},
-{VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE,
+{VIR_DOMAIN_MEMORY_SOFT_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE,
N_(Memory during contention in kilobytes)},
-{VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_STRING, VSH_OFLAG_NONE,
+{VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VSH_OT_INT, VSH_OFLAG_NONE,
N_(Max swap in kilobytes)},
-{VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_STRING, VSH_OFLAG_NONE,
+{VIR_DOMAIN_MEMORY_MIN_GUARANTEE, VSH_OT_INT, VSH_OFLAG_NONE,
N_(Min guaranteed memory in kilobytes)},
{NULL, 0, 0, NULL}
};
@@ -3458,7 +3458,7 @@ static const vshCmdInfo info_migrate_setmaxdowntime[] = {
static const vshCmdOptDef opts_migrate_setmaxdowntime[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
-{downtime, VSH_OT_DATA, VSH_OFLAG_REQ, N_(maximum tolerable downtime (in
milliseconds) for migration)},
+{downtime, VSH_OT_INT, VSH_OFLAG_REQ, N_(maximum tolerable downtime (in
milliseconds) for migration)},
{NULL, 0, 0, NULL}
};
@@ -10006,7 +10006,8 @@ vshCmddefGetData(const vshCmdDef * cmd, int data_ct)
const vshCmdOptDef *opt;
for (opt = cmd-opts; opt opt-name; opt++) {
-if (opt-type= VSH_OT_DATA) {
+if (opt-type= VSH_OT_DATA ||
+(opt-type == VSH_OT_INT (opt-flag VSH_OFLAG_REQ))) {
if (data_ct == 0 || opt-type == VSH_OT_ARGV)
return opt;
else
@@ -10089,7 +10090,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
Re: [libvirt] [PATCHv2] virsh: fix range of memtune command
On 10/20/2010 03:29 PM, Eric Blake wrote:
* tools/virsh.c (cmdMemtune): Use long long for memory
sizes. Simplify allocation, and plug memory leak.
---
No need to cripple virsh with a 32-bit limit.
Change from v1:
fix compilation failure, rebase on top of other recent memtune fixes
tools/virsh.c | 19 ---
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ca9a61e..6a11a7a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2905,7 +2905,7 @@ static int
cmdMemtune(vshControl * ctl, const vshCmd * cmd)
{
virDomainPtr dom;
-int hard_limit, soft_limit, swap_hard_limit, min_guarantee;
+long long hard_limit, soft_limit, swap_hard_limit, min_guarantee;
int nparams = 0;
unsigned int i = 0;
virMemoryParameterPtr params = NULL, temp = NULL;
@@ -2918,24 +2918,22 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
return FALSE;
hard_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_HARD_LIMIT,hard_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_HARD_LIMIT, NULL);
if (hard_limit)
nparams++;
soft_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_SOFT_LIMIT,soft_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_SOFT_LIMIT, NULL);
if (soft_limit)
nparams++;
swap_hard_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
-swap_hard_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, NULL);
if (swap_hard_limit)
nparams++;
min_guarantee =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
-min_guarantee);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, NULL);
if (min_guarantee)
nparams++;
@@ -2954,8 +2952,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
}
/* now go get all the memory parameters */
-params = vshMalloc(ctl, sizeof(virMemoryParameter) * nparams);
-memset(params, 0, sizeof(virMemoryParameter) * nparams);
+params = vshCalloc(ctl, nparams, sizeof(*params));
if (virDomainGetMemoryParameters(dom, params,nparams, 0) != 0) {
vshError(ctl, %s, _(Unable to get memory parameters));
goto cleanup;
@@ -2995,9 +2992,8 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
ret = TRUE;
} else {
/* set the memory parameters */
-params = vshMalloc(ctl, sizeof(virMemoryParameter) * nparams);
+params = vshCalloc(ctl, nparams, sizeof(*params));
-memset(params, 0, sizeof(virMemoryParameter) * nparams);
for (i = 0; i nparams; i++) {
temp =params[i];
temp-type = VIR_DOMAIN_MEMORY_PARAM_ULLONG;
@@ -3037,6 +3033,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
}
cleanup:
+VIR_FREE(params);
virDomainFree(dom);
return ret;
}
ACK.
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: work around dash 0.5.5 bug in managed save
On 10/22/2010 07:29 PM, Eric Blake wrote:
* configure.ac (VIR_WRAPPER_SHELL): Define to a replacement shell,
if /bin/sh is broken on redirection.
* src/qemu/qemu_monitor.h (VIR_WRAPPER_SHELL_PREFIX)
(VIR_WRAPPER_SHELL_SUFFIX): New macros.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Use
them.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile):
Likewise.
---
Took longer than I thought, but this should do the trick with no
overhead on decent systems, and while still avoiding buggy dash ==
/bin/sh with something that works elsewhere.
This passes for me on Fedora, where /bin/sh is bash; testing on
Ubuntu or other debian-based system where /bin/sh is dash 0.5.5
would be appreciated.
configure.ac | 50 ++
src/qemu/qemu_monitor.h | 13 +++
src/qemu/qemu_monitor_json.c |5 ++-
src/qemu/qemu_monitor_text.c |5 ++-
4 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index e41f2b5..6aa09f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -583,6 +583,56 @@ if test $with_lxc = yes ; then
fi
AM_CONDITIONAL([WITH_LXC], [test $with_lxc = yes])
+dnl
+dnl check for shell that understands redirection without truncation,
+dnl needed by src/qemu/qemu_monitor_{text,json}.c.
+dnl
+if test $with_qemu = yes; then
+ lv_wrapper_shell=
+ AC_CACHE_CHECK([for shell that supports redirection],
+[lv_cv_wrapper_shell],
+[
+# If cross-compiling, guess that /bin/sh is good enough except for
+# Linux, where it might be dash 0.5.5 which is known broken; and on
+# Linux, we have a good chance that /bin/bash will exist.
+# If we guess wrong, a user can override the cache variable.
+# Going through /bin/bash is a slight slowdown if /bin/sh works.
+if test $cross_compiling = yes; then
+ case $host_os in
+linux*) lv_cv_wrapper_shell=/bin/bash ;;
+*) lv_cv_wrapper_shell=/bin/sh ;;
+ esac
+else
+ for lv_cv_wrapper_shell in /bin/sh bash ksh zsh none; do
+test $lv_cv_wrapper_shell = none
+ AC_MSG_ERROR([could not find decent shell])
+echo a conftest.a
+$lv_cv_wrapper_shell -c ': 1conftest.a'
+case `cat conftest.a`.$lv_cv_wrapper_shell in
+ a./*) break;; dnl /bin/sh is good enough
+ a.*) dnl bash, ksh, and zsh all understand 'command', use that
+ dnl to determine the absolute path of the shell
+lv_cv_wrapper_shell=`$lv_cv_wrapper_shell -c \
+ 'command -v $lv_cv_wrapper_shell'`
+case $lv_cv_wrapper_shell in
+ /*) break;;
+esac
+;;
+esac
+ done
+ rm -f conftest.a
+fi
+ ])
+ if test x$lv_cv_wrapper_shell != x/bin/sh; then
+lv_wrapper_shell=$lv_cv_wrapper_shell
+ fi
+ if test x$lv_wrapper_shell != x; then
+AC_DEFINE_UNQUOTED([VIR_WRAPPER_SHELL], [$lv_wrapper_shell],
+ [Define to the absolute path of a shell that does not truncate on
+ redirection, if /bin/sh does not fit the bill])
+ fi
+fi
+
dnl
dnl check for kernel headers required by src/bridge.c
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 48f4c20..7d09145 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -391,4 +391,17 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const
char *name);
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char
**reply);
+/**
+ * When running two dd process and using redirection, we need a
+ * shell that will not truncate files. These two strings serve that
+ * purpose.
+ */
+# ifdef VIR_WRAPPER_SHELL
+# define VIR_WRAPPER_SHELL_PREFIX VIR_WRAPPER_SHELL -c '
+# define VIR_WRAPPER_SHELL_SUFFIX '
+# else
+# define VIR_WRAPPER_SHELL_PREFIX /* nothing */
+# define VIR_WRAPPER_SHELL_SUFFIX /* nothing */
+# endif
+
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d3ab25f..d2c6f0a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1698,8 +1698,9 @@ int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon,
* allow starting at an alignment of 512, but without wasting
* padding to get to the larger alignment useful for speed. Use
* redirection to avoid truncating a regular file. */
-if (virAsprintf(dest, exec:%s | { dd bs=%llu seek=%llu if=/dev/null
-dd bs=%llu; } 1%s,
+if (virAsprintf(dest, exec: VIR_WRAPPER_SHELL_PREFIX %s |
+{ dd bs=%llu seek=%llu if=/dev/null
+dd bs=%llu; } 1%s VIR_WRAPPER_SHELL_SUFFIX,
argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS,
offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS,
QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 69971a6..d7e128c 100644
---
Re: [libvirt] [PATCH] qemu: work around dash 0.5.5 bug in managed save
On 10/23/2010 10:29 AM, Eric Blake wrote: * configure.ac (VIR_WRAPPER_SHELL): Define to a replacement shell, if /bin/sh is broken on redirection. * src/qemu/qemu_monitor.h (VIR_WRAPPER_SHELL_PREFIX) (VIR_WRAPPER_SHELL_SUFFIX): New macros. * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Use them. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile): Likewise. --- Took longer than I thought, but this should do the trick with no overhead on decent systems, and while still avoiding buggy dash == /bin/sh with something that works elsewhere. This passes for me on Fedora, where /bin/sh is bash; testing on Ubuntu or other debian-based system where /bin/sh is dash 0.5.5 would be appreciated. No obvious errors crop up when testing this on Ubuntu 10.04: virsh # managedsave 1 Domain 1 state saved by libvirt virsh # As a usability thing though, there doesn't seem to be a matching managedrestore command in virsh, and the help managedsave command is bit light on how do we restart a saved domain?: # help managedsave NAME managedsave - managed save of a domain state SYNOPSIS managedsave domain DESCRIPTION Save and stop a running domain, so libvirt can restart it from the same state OPTIONS [--domain] string domain name, id or uuid Are we supposed to restart it using start or something? (we need to mention this, which I can follow up with a patch) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: revamp api_extension example, using vcpu patch series
On 10/20/2010 12:19 PM, Eric Blake wrote: Complete patch is compressed and attached, because it is so large. But the bulk of it consisted mainly of running 'git format-patch -15 b0137887' to pick up the top of my vcpu series, then replacing the contents of docs/api_extension/ with the new patches. The meat of the patch is the overview html file, copied here if you don't want to unzip the attachment: Some suggestions below. 2010-10-20 Eric Blake ebl...@redhat.com docs: revamp api_extension example, using vcpu patch series * docs/api_extension/*: Replace example files. * docs/api_extension.html.in: Rewrite to match new example files. diff --git c/docs/api_extension.html.in w/docs/api_extension.html.in index de6eedc..1396e91 100644 --- c/docs/api_extension.html.in +++ w/docs/api_extension.html.in @@ -10,8 +10,12 @@ p This document walks you through the process of implementing a new - API in libvirt. It uses as an example the addition of the node device - create and destroy APIs. + API in libvirt. It uses as an example the addition of ... the addition of an API for separating maximum.. ? + separating maximum from current vcpu usage of a domain. + Remember that new API includes any new public functions, but it that a new API + also includes addition of flags or extension of XML used through may include addition of flags or extensions of XML used by + existing functions. The example in this document adds both + types of API, and not all extensions require quite as many patches. I'd make a new sentence after the comma. Maybe: Also, not all libvirt extensions require quite as many patches. /p p @@ -23,7 +27,12 @@ added to libvirt. Someone may already be working on the feature you want. Also, recognize that everything you write is likely to undergo significant rework as you discuss it with the other developers, so - don't wait too long before getting feedback. + don't wait too long before getting feedback. In the vcpu example + below, list feedback was first requested + a href=https://www.redhat.com/archives/libvir-list/2010-September/msg00423.html;here/a, + and resulted in several rounds of improvements before coding + began, and this example is slightly rearranged from the actual + order of the commits. New sentence after the 'and'. /p p @@ -46,11 +55,22 @@ lidefine the public API/li lidefine the internal driver API/li liimplement the public API/li - lidefine the wire protocol format/li - liimplement the RPC client/li - liimplement the server side dispatcher/li - liimplement the driver methods/li + liimplement the remote protocol: + ol + lidefine the wire protocol format/li + liimplement the RPC client/li + liimplement the server side dispatcher/li + /ol + /li + liuse new API where appropriate in drivers/li liadd virsh support/li + liadd common handling for new API/li + lifor each driver that can support the new API: + ol + liadd prerequisite support/li + lifully implement new API/li + /ol + /li /ol p @@ -66,11 +86,10 @@ functionality--get the whole thing working and make sure you're happy with it. Then use git or some other version control system that lets you rewrite your commit history and break patches into pieces so you - don't drop a big blob of code on the mailing list at one go. For - example, I didn't follow my own advice when I originally submitted the - example code to the libvirt list but rather submitted it in several - large chunks. I've used git's ability to rewrite my commit history to - break the code apart into the example patches shown. + don't drop a big blob of code on the mailing list at one go. in one go + The example code used in this document was rebased several + times between the original list posting and the final committed + version. I'd say that the implementer has to follow the tree and adapt the patches as the tree changes. /p p @@ -86,9 +105,23 @@ h2a name='publicapi'Defining the public API/a/h2 - pThe first task is to define the public API and add it to:/p + pThe first task is to define the public API. If the addition If the extension requires a new XML, you must enhance the RelaxNG schema and document the new elements or attributes. + involves XML, this includes enhancing the RelaxNG schema and + documenting the new elements or attributes:/p - pcodeinclude/libvirt/libvirt.h.in/code/p + pcode +docs/schemas/domain.rngbr/ +docs/formatdomain.html.in + /code/p + + pIf the addition involves a new function, this involves adding + a declaration in the public header, and arranging to export the + symbol:/p Some how I don't like 'addition'. I'd say 'libvirt extension'. If the extension adds a new function you have to add a declaration in the public header file as well as export the
[libvirt] How to start a defined domain
I've create a domain called vm01 using Virt-Manager, the GUI tool. Now I wanna use libvirt api to control the life cycle of vm01. It went on well with shutdown and reboot, but how to start it became a problem. I was intended to get the virDomainPtr by using virDomainLookupByName api, and then start it using virDomainCreate api. But it turns out that virDomainLookupByName api cannot return a virDomainPtr for a defined but not running domain. That's my situation, what do you suggest I do now? Thanks in advance. 2010-10-26 Lancer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] --checksum-fill error on Ubuntu 10.04
caused by network filter driver depends on the newer iptables, there is no --checksum-fill option indeed in older iptables. The following commit log is from iptables git. commit 9d1b11102b53103c00b7fddf4658a4d2bdee1338 Author: Michael S. Tsirkin m...@redhat.com Date: Thu Jul 15 17:23:24 2010 +0200 extensions: libxt_CHECKSUM extension This adds a `CHECKSUM' target, which can be used in the iptables mangle table. You can use this target to compute and fill in the checksum in a packet that lacks a checksum. This is particularly useful, if you need to work around old applications such as dhcp clients, that do not work well with checksum offloads, but don't want to disable checksum offload in your device. The problem happens in the field with virtualized applications. For reference, see Red Hat bz 60, as well as http://www.spinics.net/lists/kvm/msg37660.html Typical expected use (helps old dhclient binary running in a VM): iptables -A POSTROUTING -t mangle -p udp --dport bootpc \ -j CHECKSUM --checksum-fill Includes fixes by Jan Engelhardt jeng...@medozas.de Signed-off-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Patrick McHardy ka...@trash.net Even on my FC13, it's without CHECKSUM support, the build date is 'Thu 08 Apr 2010 06:30:45 PM CST', :-) Could we treat the error message as warnings? or make it only works for newer iptables? - Osier - Justin Clift jcl...@redhat.com wrote: Hi us, Just noticed an error message when manually starting libvirtd (built from git) on Ubuntu 10.04, while checking something else: iptables v1.4.4: unknown option `--checksum-fill' It shows up in the initial default network setup: 10:40:19.807: error : virRunWithHook:855 : internal error '/sbin/iptables --table mangle --insert POSTROUTING --out-interface virbr0 --protocol udp --destination-port 68 --jump CHECKSUM --checksum-fill' exited with non-zero status 2 and signal 0: iptables v1.4.4: unknown option `--checksum-fill' Try `iptables -h' or 'iptables --help' for more information. Just a FYI, in case someone wants to look into it. Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] How to start a defined domain
- 黄亮 lancerhu...@163.com wrote: I've create a domain called vm01 using Virt-Manager, the GUI tool. Now I wanna use libvirt api to control the life cycle of vm01. It went on well with shutdown and reboot, but how to start it became a problem. I was intended to get the virDomainPtr by using virDomainLookupByName api, and then start it using virDomainCreate api. But it turns out that virDomainLookupByName api cannot return a virDomainPtr for a defined but not running domain. no, it can :-) If it really doesn't work for you, could you write a simple program to reproduce it? and try to explain what's the version of libvirt you use, on which distro, etc, any helpful info is welcomed. :-) Thanks - Osier That's my situation, what do you suggest I do now? Thanks in advance. 2010-10-26 Lancer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] virsh: fix range of memtune command
On Wed, 20 Oct 2010 13:29:45 -0600, Eric Blake ebl...@redhat.com wrote:
* tools/virsh.c (cmdMemtune): Use long long for memory
sizes. Simplify allocation, and plug memory leak.
---
No need to cripple virsh with a 32-bit limit.
Change from v1:
fix compilation failure, rebase on top of other recent memtune fixes
tools/virsh.c | 19 ---
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ca9a61e..6a11a7a 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2905,7 +2905,7 @@ static int
cmdMemtune(vshControl * ctl, const vshCmd * cmd)
{
virDomainPtr dom;
-int hard_limit, soft_limit, swap_hard_limit, min_guarantee;
+long long hard_limit, soft_limit, swap_hard_limit, min_guarantee;
int nparams = 0;
unsigned int i = 0;
virMemoryParameterPtr params = NULL, temp = NULL;
@@ -2918,24 +2918,22 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
return FALSE;
hard_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_HARD_LIMIT, NULL);
if (hard_limit)
nparams++;
soft_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_SOFT_LIMIT, NULL);
if (soft_limit)
nparams++;
swap_hard_limit =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
- swap_hard_limit);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, NULL);
if (swap_hard_limit)
nparams++;
min_guarantee =
-vshCommandOptInt(cmd, VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
- min_guarantee);
+vshCommandOptLongLong(cmd, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, NULL);
if (min_guarantee)
nparams++;
@@ -2954,8 +2952,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
}
/* now go get all the memory parameters */
-params = vshMalloc(ctl, sizeof(virMemoryParameter) * nparams);
-memset(params, 0, sizeof(virMemoryParameter) * nparams);
+params = vshCalloc(ctl, nparams, sizeof(*params));
if (virDomainGetMemoryParameters(dom, params, nparams, 0) != 0) {
vshError(ctl, %s, _(Unable to get memory parameters));
goto cleanup;
@@ -2995,9 +2992,8 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
ret = TRUE;
} else {
/* set the memory parameters */
-params = vshMalloc(ctl, sizeof(virMemoryParameter) * nparams);
+params = vshCalloc(ctl, nparams, sizeof(*params));
-memset(params, 0, sizeof(virMemoryParameter) * nparams);
for (i = 0; i nparams; i++) {
temp = params[i];
temp-type = VIR_DOMAIN_MEMORY_PARAM_ULLONG;
@@ -3037,6 +3033,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
}
cleanup:
+VIR_FREE(params);
virDomainFree(dom);
return ret;
}
ACK
Nikunj
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
