November 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] Set owner and group for file used for saving domain
by Anoop Vijayan 11 Nov '09

11 Nov '09

virsh save a domain created by libvirt and it hangs with the log message "sh: /home/newguest: Permission denied". --- src/qemu/qemu_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 30003e6..a2d9534 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3415,6 +3415,11 @@ static int qemudDomainSave(virDomainPtr dom, goto endjob; } + if (qemuDomainSetFileOwnership(dom->conn, path, driver->user, + driver->group) < 0 ) { + goto endjob; + } + if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to write save header")); -- 1.5.4.5 -- Cheers! Anoop Linux Technology Center, IBM

2 1

[libvirt] [PATCH] Fix warning on make due to missing cast (int)
by Ryota Ozaki 11 Nov '09

11 Nov '09

qemu/qemu_monitor_text.c: In function 'qemuMonitorTextIOProcess': qemu/qemu_monitor_text.c:175: error: format '%d' expects type 'int', but argument 7 has type 'size_t' * src/qemu/qemu_monitor.c src/qemu/qemu_monitor_text.c: cast size_t to int when passing to '%d' --- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor_text.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 3e6a490..dcd2dd7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -179,7 +179,7 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) if (mon->msg && mon->msg->txOffset == mon->msg->txLength) msg = mon->msg; - VIR_DEBUG("Process %d", mon->bufferOffset); + VIR_DEBUG("Process %d", (int)mon->bufferOffset); len = qemuMonitorTextIOProcess(mon, mon->buffer, mon->bufferOffset, msg); @@ -196,7 +196,7 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) VIR_FREE(mon->buffer); mon->bufferOffset = mon->bufferLength = 0; } - VIR_DEBUG("Process done %d used %d", mon->bufferOffset, len); + VIR_DEBUG("Process done %d used %d", (int)mon->bufferOffset, len); if (msg && msg->finished) virCondBroadcast(&mon->notify); return len; @@ -319,7 +319,7 @@ qemuMonitorIORead(qemuMonitorPtr mon) mon->buffer[mon->bufferOffset] = '\0'; } - VIR_DEBUG("Now read %d bytes of data", mon->bufferOffset); + VIR_DEBUG("Now read %d bytes of data", (int)mon->bufferOffset); return ret; } diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index db7ff57..ee234a1 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -172,7 +172,7 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, /* Don't print raw data in debug because its full of control chars */ /*VIR_DEBUG("Process data %d byts of data [%s]", len - used, data + used);*/ - VIR_DEBUG("Process data %d byts of data", len - used); + VIR_DEBUG("Process data %d byts of data", (int)(len - used)); /* Look for a non-zero reply followed by prompt */ if (msg && !msg->finished) { -- 1.6.2.5

3 2

[libvirt] [PATCH] Prevent initializing ebtables if disabled in qemu.conf
by Ryota Ozaki 11 Nov '09

11 Nov '09

* src/qemu/qemu_conf.c: don't initialize ebtables if disabled --- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 28567b2..5f492dd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -322,7 +322,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, p = virConfGetValue (conf, "mac_filter"); CHECK_TYPE ("mac_filter", VIR_CONF_LONG); - if (p) { + if (p && p->l) { driver->macFilter = p->l; if (!(driver->ebtables = ebtablesContextNew("qemu"))) { driver->macFilter = 0; -- 1.6.2.5

5 5

[libvirt] RFC move logging initialization earlier in libvirtd
by Dave Allan 11 Nov '09

11 Nov '09

I noticed yesterday that logging gets initialized fairly far into the libvirtd startup process, so messages generated during server startup don't get logged to the same sinks that will be used for the rest of the life of the daemon. I'd like to explore moving the logging startup to an earlier point. Has this idea been explored previously? Dave

2 3

[libvirt] [PATCH] phyp: ssh authentication with pub keys fixed
by Eduardo Otubo 11 Nov '09

11 Nov '09

-- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 eotubo(a)linux.vnet.ibm.com

2 9

[libvirt] ANNOUNCE: libguestfs 1.0.78 released
by Richard W.M. Jones 11 Nov '09

11 Nov '09

I'm pleased to announce the release of libguestfs 1.0.78. Libguestfs is a library for accessing and modifying virtual machine disk images. Home page: http://libguestfs.org/ Source: http://libguestfs.org/download/ Binaries: http://libguestfs.org/FAQ.html#binaries A Fedora build is available in Koji here: http://koji.fedoraproject.org/koji/packageinfo?packageID=8391 (These release notes cover all the significant changes since the last announcement which was for 1.0.67, 2 months ago). New features: - FUSE support so you can mount guest filesystems in the host: http://rwmj.wordpress.com/2009/11/03/browsing-guests-using-fuse/ - Support for btrfs, gfs, gfs2, hfs, hfs+, nilfs2, jfs, reiserfs, xfs: http://rwmj.wordpress.com/2009/11/08/filesystem-metadata-overhead/ - Support for huge (multi-exabyte) sparse virtual disks: http://rwmj.wordpress.com/2009/11/04/petabytes-exabytes-why-not/ http://git.et.redhat.com/?p=libguestfs.git;a=commit;h=5ce72e039ca332ba19bb9… - New partitioning API, supports GPT and more: http://libguestfs.org/guestfs.3.html#guestfs_part_add http://git.et.redhat.com/?p=libguestfs.git;a=commit;h=b1e1ca2f74a921b3f7845… - New tool: virt-ls http://rwmj.wordpress.com/2009/10/19/new-tool-virt-ls/ - New tool: virt-tar http://rwmj.wordpress.com/2009/10/19/new-tool-virt-tar/ - New tool: virt-edit http://rwmj.wordpress.com/2009/09/23/virt-edit/ - New tool: virt-rescue http://rwmj.wordpress.com/2009/09/22/virt-rescue/ - Windows Registry support, tools and library: http://rwmj.wordpress.com/2009/10/29/virt-win-reg-get-at-the-windows-regist… http://rwmj.wordpress.com/2009/10/28/libhivex-windows-registry-hive-extract… - OCaml bindings for virt-inspector - RELAX NG schema for virt-inspector - New APIs: utimens, vfs_type, truncate, truncate_size, lchown, lstatlist, lxattrlist, readlinklist, case_sensitive_path, find0, mkfs_b, mke2journal, and more ... - New program: OCaml viewer http://rwmj.wordpress.com/2009/09/29/graphical-virt-df/ - Allow stdout to be redirected when running guestfish remotely (Matt Booth). - Remove requirement for vmchannel support in qemu (horray!) and the tricky main loop code. Bug fixes: (Too many to list here, see: http://git.et.redhat.com/?p=libguestfs.git;a=log) Thanks to: Jim Meyering, Matt Booth and Charles Duffy for lots of bugfixes, patches and testing. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html

1 0

[libvirt] Re: libvirt code review
by Daniel Veillard 11 Nov '09

11 Nov '09

On Thu, Nov 05, 2009 at 03:27:16PM -0500, Steve Grubb wrote: > Hello, > > I missed code in a couple other directories. This is much shorter. > > -Steve > > > In daemon/libvirtd.c at line 371 the "if" statement will never be true. ret is > set to 0 at line 350 and never changed. probably a leftover from previous refactoring, server->shutdown = 1 is directly set in the cases that matters, removed ret altogether. In the current code head, server->shutdown has been renamed server->quitEventThread, so patch changed accordingly. > At line 1535 seems to want to loop on reads that return -1 and either EINTR or > EAGAIN. The implementation does not. Rather than calling return at line 1538, > it should likely have a "goto again" with the again label placed just before > the read call. Actually in that case I think the current code is fine, the function should exit 0 on EAGAIN, it's documented, I assume the read will be retried by the caller, otherwise we would have used saferead() there an internal routine taking directly care of interrupted calls. > At line 1783 is a call to write that likely wants a treatment similar to the > read above. Same thing, I guess it's intended behaviour :-) > In tools/virsh.c at line 4616 is a test that format is true. It was checked at > line 4614 and nothing changed it. Ah right, harmless but fixed :-) > At line 6810 memory is allocated. At the error exit at line 6824 it has not > been freed. okay > At line 7664 is a test for network==NULL. This will always be true. > At line 7704 is a test for iface == NULL. This will always be true. > At line 7741 is a test for pool==NULL. This will always be true. Okay, fixed, allows the test to fit in 80 columns too, Patch for git head attached. thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 1

[libvirt] [PATCH] Bump required netcf version to 0.1.4
by Laine Stump 11 Nov '09

11 Nov '09

0.1.3 will build everything, but causes a segfault. --- configure.in | 2 +- libvirt.spec.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index 7ad1a90..cd8bb95 100644 --- a/configure.in +++ b/configure.in @@ -33,7 +33,7 @@ GNUTLS_REQUIRED="1.0.25" AVAHI_REQUIRED="0.6.0" POLKIT_REQUIRED="0.6" PARTED_REQUIRED="1.8.0" -NETCF_REQUIRED="0.1.3" +NETCF_REQUIRED="0.1.4" dnl Checks for C compiler. AC_PROG_CC diff --git a/libvirt.spec.in b/libvirt.spec.in index 9238bdd..7c43180 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -308,7 +308,7 @@ BuildRequires: libcap-ng-devel >= 0.5.0 BuildRequires: libssh2-devel %endif %if %{with_netcf} -BuildRequires: netcf-devel >= 0.1.3 +BuildRequires: netcf-devel >= 0.1.4 %endif # Fedora build root suckage -- 1.6.5.15.gc274d

2 1

[libvirt] Re: libvirt code review
by Daniel Veillard 11 Nov '09

11 Nov '09

On Thu, Nov 05, 2009 at 01:12:41PM -0500, Steve Grubb wrote: > Hello, > > The following is against 0.7.2-3 from F-13 cvs. I mention each source file at > the beginning of a block and have a blank line indicating that we are moving > along to the next file. Hope you find this helpful. > > -Steve Okay I fixed the issues on a 0.7.2 branch and then backported the patch back onto git head, except for a few exceptions, everything was still applying ! > In src/util/storage_file.c at line 192, there is a test that I presume is for > -1. The problem is that its a variable that is an unsigned long is 64 bits on unsigned long size; size = ((buf[4+4+8] << 24) | (buf[4+4+8+1] << 16) | (buf[4+4+8+2] << 8) | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ [...] if (size + 1 == 0) return BACKING_STORE_INVALID; if (VIR_ALLOC_N(*res, size + 1) < 0) { I think it's just to make 100% sure the size + 1 computation on the following line won't overflow. So current code looks fine to me > some arches. It would probably be better to use a unit32_t for size or change > the test. The bottom line is the test will evaluate false on some arches. unit32_t is nowhere used in libvirt code I would rather not use it, we could use size_t there since the goal is to allocate memory but I'm not 100% sure it's right, basically the test make sure we won't try to allocate more than 4G of memory in one chunk. > In src/datatypes.c at lines 335, 476, and 790, there is a test to see if uuid > is NULL. There was a test in the beginning of the function and it returned if > this was true, so this "if" statement can be deleted in all 3 cases. Hum, this is the virGetxxx() functions, they lookup objects given the name or uuid, and hold big TODOs comments at the start: /* TODO search by UUID first as they are better differenciators */ /* TODO check the UUID */ And somehow I think the intend was to allow lookup only by name, with the uuid being unknown, I would think the proper fix is actually to drop the || (uuid == NULL) at the start of the function. > In src/libvirt.c at line 1122, there is an "if" statement that "ands" 0 with > the return from STREQ. Should that have been a == ? The test evaluates false > as is. Comparing with others similar message being emitted in other places, I think '0 && ' is just a bogus remain, I'm removing it. > At line 3214, there is a test if uri == NULL. Then within that "if" > statement at line 3216 is a test for if uri is still NULL. Should that have > been a test for dstURI being NULL? yup ! That had been fixed in head since though. > In src/remote/remote_driver.c at line 939 is a do loop. By design, it seems > the intention was to reloop if waitpid returns -1 and EINTR is set. The > implementation doesn't match it. If -1 is returned, it will do the continue, > which is to jump to the end of the do loop and it evaluates reap which would > be a -1 and it will exit. I suspect instead of continue, it should "goto > again" and again would be placed before the call to waitpid. > This problem is found again at line 1406 okay, nasty, somehow it's easy to think continue will retry from top of the loop not including the test > In src/xen/sexpr.c at line 443 is a test for token == NULL. Token is > guaranteed to not be NULL because of the loop entry test at line 440. okay > In src/xen/xend_internal.c at line 1367 is a test for connectPort being true. > Prior to this, connectPort is guranteed to be valid or it would have jumped to > no_memory. okay as the affectation is done in both if and else parts of the preceeding test. > In src/xen/xm_internal.c at line 579 is a test for dh being valid. It is > guaranteed to be valid since a -1 was returned from the function on a failed > call to opendir. okay > At line 2219 is an unusual if statement. Normally you do not see something > constructed as (!cpu)<0). That would seem to have meant cpu>=0 which is more > straightforward. if (def->cpumask && !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0) goto cleanup; But the function returns a string or NULL in case of error, I try to fight again not fully parenthetized test expressions but I'm loosing that fight unfortunately :-( in any case the < 0 test is bogus since it's a string if ((def->cpumask) && ((cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) == NULL)) goto cleanup seems the right construct to me, as it tests if the call failed. > In src/phyp/phyp_driver.c at line 1008, there is a strdup inside an if > statement with nothing catching its returned memory. Two lines later it does > the same thing again but puts it in an array. Not sure what the if statement > should have been checking, but its definitely a memory leak as is. > At line 1040 is an allocation. If that fails, it passes dom->comm to an error > reporting function. However, at line 1034, dom was set to NULL and is not > changed. > The same problem appears again at line 1085. > At line 1105 is a test for exit_status < 0. Nothing has changed it since it > was initialized. That code was completely revamped since, none of this applies now, it was pretty bad ! C.f. https://bugzilla.redhat.com/show_bug.cgi?id=529977 > In src/openvz/openvz_conf at line 336 is a test for from being NULL. However, > it was previously used at line 333 before checking if it was NULL. yeah, there was another passed pointer which should have got the same check too, cleaning this up. > In src/qemu/qemu_monitor_text.c at line 178, a variable, control, is assigned > to a member of the msg structure. However, control goes out of scope before > its used. It should be in the function level declarations. Ah right, nasty ! > In src/qemu/qemu_driver.c at line 894 is a test for retries being 0. This will > always be true since the loop has no break statements. > At line 4484, we leave the function without freeing devname. okay, > In src/lxc/lxc_conf.c at line 111, 114, and 125, we leave the function without > freeing filename. okay > In src/lxc/lxc_container.c at line 743, we leave the function without freeing > ttyPath. okay > In src/storage/storage_driver.c at lines 83 and 93 we call storageLog/fprintf > with the second parameter to %s being NULL. Wasn't an empty string intended? I used "no error message found" as we failed to gather an error message > In src/storage/storage_backend_disk.c at line 88, we leave the function > without freeing devpath. okay > At line 649 is a test for part_num being NULL. Can it ever be NULL? No, but we should check for an empty string, done > In src/node_device/node_device_hal.c at line 405 is a test for hal_cap_names > being true, its conceivable to get there without it being initialized. For > example, line 373 could jump to it. It should probably be initialized to 0. okay > At line 781 is a test for udi being true. It will alwys be false since once > its defined it will never jump to failure. > At line 782 is a use of num_devs without it being initialized. If the problem > mentioned for udi is fixed by deleting all this code, then the problem goes > away. okay removing that block, I just hope we won't add a failure jump from within the loop > In src/lxc/lxc_controller.c at line 633 is a test for containerPty not being > -1. Its conceivable to get here with it notr being initialized perhaps at line > 512 for example. It should probably be initialized to a -1. yup, okay > At line 735 is an if statement "anding" 0 with getuid(). this should probably > be a != rather than a &&. right since the error message clearly indicate we expect this to run as root. Thanks a lot for the review ! Patch for git head attached Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 5

[libvirt] Mounting a raw DVD
by Adam Mooz 11 Nov '09

11 Nov '09

Hello List, I'm attempting to pass a DVD drive of a headless server into a virtual machine. So far I've managed to mount *something* into the VM, but it's being registered as a blank DVD within the VM. When I mount /dev/ dvd into the bare-metal machine it registers normally as the DVD. How do I pass physical drives into a VM and back out using libvirt? Command used: #virsh attach-disk Montreal /dev/dvd hdc --mode readonly --driver phy --type cdrom Connecting to uri: qemu:///system Disk attached successfully VM XML file: <domain type='kvm'> <name>Montreal</name> <uuid>0f79ba7f-27bf-16a0-5887-6cfa2b5569f1</uuid> <memory>2097152</memory> <currentMemory>2097152</currentMemory> <vcpu>1</vcpu> <os> <type arch='x86_64' machine='pc-0.11'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/bin/kvm</emulator> <disk type='file' device='cdrom'> <target dev='hdc' bus='ide'/> <readonly/> </disk> <disk type='file' device='disk'> <source file='(Mangled Purposely)'/> <target dev='vda' bus='virtio'/> </disk> <interface type='bridge'> <mac address='(mangled purposly)/> <source bridge='br0'/> <model type='virtio'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target port='0'/> </console> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/> <video> <model type='cirrus' vram='9216' heads='1'/> </video> </devices> </domain> Thanks in advance, ----------------------------------------------------------------- Adam Mooz Adam.Mooz(a)gmail.com AdamMooz(a)me.com http://www.AdamMooz.com

2 5