November 2015 - Libguestfs - libguestfs List Archives

DATA LOSS BUG: Please DON'T use libguestfs 1.30.5
by Richard W.M. Jones 16 Dec '15

16 Dec '15

libguestfs 1.30.5 (and current upstream development version) has a serious bug in virt-resize where logical partitions get deleted. For details, see: https://bugzilla.redhat.com/show_bug.cgi?id=1285847 In brief: don't use version 1.30.5 (1.30.4 is OK). I have removed the tarball from the downloads directory. I'll try to get 1.30.6 out with a fix as soon as possible. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

1 1

Plan for libguestfs 1.30
by Richard W.M. Jones 14 Dec '15

14 Dec '15

It has been a ludicrously long time (over 8 months) since the last stable release of libguestfs. So I'd like to plan a new 1.30 release soon. Please follow-up if there are features / blockers / bugs that need to be addressed for 1.30. As usual, bugs which have "1.30" (without quotes) in the Devel Whiteboard field in Bugzilla are nominated as blockers for the release. You can see a list of those here [currently empty]: https://bugzilla.redhat.com/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&… For a list of all bugs, go to: https://bugzilla.redhat.com/buglist.cgi?component=libguestfs&product=Virtua… ---------------------------------------------------------------------- It's also worth talking about what I'd like to do *after* 1.30 is out. This includes: - threading support https://www.redhat.com/archives/libguestfs/2015-June/thread.html#00118 - external tests https://www.redhat.com/archives/libguestfs/2014-October/thread.html#00042 While thread safety is oft talked about, it's external tests which I consider to be the more important patch series, since it will allow us to build and test libguestfs more easily and more comprehensively for Fedora and RHEL. Unfortunately it's the most difficult and invasive change of the two. You may have your own ideas, if so follow up here. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

3 6

SELinux relabel API
by Richard W.M. Jones 10 Dec '15

10 Dec '15

[ I realized that we were discussing adding this feature, in various private email, IRC, and this long bugzilla thread: https://bugzilla.redhat.com/show_bug.cgi?id=1060423 That's not how we should do things. Let's discuss it on the mailing list. ] One thing that virt-customize/virt-sysprep/virt-builder have to do is relabel SELinux guests. What we do at the moment is run: if load_policy && fixfiles restore; then rm -f /.autorelabel else touch /.autorelabel echo '%s: SELinux relabelling failed, will relabel at boot instead.' fi while chrooted into the guest (using the 'guestfs_sh' API). This has a number of problems: - It has to load the policy using 'load_policy', but this doesn't work sometimes: * RHEL 5 load_policy takes a parameter. * Doesn't work if appliance kernel is significantly different from guest kernel version, because the binary policy format changes irregularly and is not backwards compatible. * Requires the appliance [host] kernel to be compiled with LSM/SELinux support. - Touching /.autorelabel is often broken, eg. it's broken in Fedora 20 because of systemd (RHBZ#1049656). - /etc/resolv.conf will not be relabelled if guestfs network is on, because of resolv.conf shenanigans in libguestfs.git/daemon/command.c - It requires running guest code, which we'd like to avoid. What would be nice would be to have an API to just do this relabelling. Libguestfs could change this API as required to handle different guests. Dan Walsh helpfully pointed out to us that we've been doing it wrong all along :-) A much better way to relabel is to run: setfiles /etc/selinux/targeted/contexts/files/file_contexts DIR where 'file_contexts' is a file which contains the default labels for files (a set of regexps), and 'DIR' is the directory at which relabelling starts. Note that 'setfiles' would be the libguestfs appliance binary, so no guest binary needs to be run. A simple API could just look like this: guestfs_selinux_relabel (g); which would always use the 'targeted' policy from the guest, and always start relabelling at the root. This would work fine for virt-builder. For Colin's requirements for Project Atomic, I suspect he will want to be able to set the file_contexts file and the root directory, but I'll leave him to describe what would be useful. A couple of notes: - I'd like to avoid baking in assumptions from the 'setfiles' command as far as possible. libguestfs APIs last for many years and some have caused us many years of regret (but that's our job) :-/ - Is it a good idea to tie this into inspection in some way -- for example, inspection could provide us with the path to the current or default SELinux policy. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

5 10

Re: [Libguestfs] Fwd: [PATCH] v2v: virtio-win: include *.dll too
by Vadim Rozenfeld 03 Dec '15

03 Dec '15

On Tue, 2015-10-27 at 11:00 -0400, Amnon Ilan wrote: > Hi Jeff, > > There were some questions on the libguestfs mailing list regarding virtio-win iso an packaging (see below). > Could you answer it on the list? (just send it to libguestfs(a)redhat.com , no need to register, keep us CCed) > The people asking are from Virtuozzo, they are not customers or official partners, just good contributors upstream. > Note that this mailing list is public. > > Thanks, > Amnon > > > > ----- Forwarded Message ----- > > From: "Richard W.M. Jones" <rjones(a)redhat.com> > > To: "Roman Kagan" <rkagan(a)virtuozzo.com>, libguestfs(a)redhat.com, "Denis Lunev" <den(a)virtuozzo.com> > > Cc: "Amnon Ilan" <ailan(a)redhat.com> > > Sent: Tuesday, October 27, 2015 4:11:10 PM > > Subject: Re: [PATCH] v2v: virtio-win: include *.dll too > > > > On Tue, Oct 27, 2015 at 04:45:00PM +0300, Roman Kagan wrote: > > > On Tue, Oct 27, 2015 at 12:02:40PM +0000, Richard W.M. Jones wrote: > > > > On Tue, Oct 27, 2015 at 02:08:42PM +0300, Roman Kagan wrote: > > > > > On Tue, Oct 27, 2015 at 09:12:41AM +0000, Richard W.M. Jones wrote: > > > > > > On Mon, Oct 26, 2015 at 09:00:03PM +0300, Roman Kagan wrote: > > > > > > > Windows QXL drivers include also qxldd.dll which used to get > > > > > > > filtered > > > > > > > out and not copied over into the guest. As a result QXL driver > > > > > > > failed > > > > > > > to install due to a missing file. > > > > > > > (* Skip files without specific extensions. *) > > > > > > > - let extensions = ["cat"; "inf"; "pdb"; "sys"] in > > > > > > > + let extensions = ["cat"; "dll"; "inf"; "pdb"; "sys"] in > > > > > > > > > > > > Actually no, this part is (may be) wrong. > > > > > > > > > > > > This causes WdfCoInstaller*.dll from the ISO to be copied in. It's > > > > > > not clear to me whether these files are needed - I suspect not. > > > > > Having WdfCoInstaller* is absolutely necessary for successful installation any of our WDF based drivers (serial, balloon, and rng). > > > > > Yes it is, see the corresponding *.inf where it's mentioned in the > > > > > corresponding CopyFiles directive (and yes, I just verified that with > > > > > that file missing the balloon driver installation fails with 'file not > > > > > found' error). > > > > > > > > Maybe we should be looking at the CopyFiles directive? (As you may be > > > > able to guess, I know next to nothing about how Windows drivers work). > > > > > > So do I, and parsing the *.inf files is the last thing I'd like to do > > > here; I'm not sure it can be made reliable (I mean, not the parsing, but > > > making any sense out of the parsed data). > > > > > > However I'm now facing a more serious problem: when I actually went > > > ahead and looked into the virio-win iso packaged in the rhel virtio-win > > > rpm, I found out that its contents differed significantly from what was > > > packaged in that very same rpm as a driver directory hierarchy. > > > > > > Namely, > > > > > > - there are no qxl drivers on the iso, but they are present in the rpm > > > > > > - there are no balloon, qemupciserial, viorng, vioserial in the rpm, > > > but they are present on the iso > > > > > > - qemupciserial is an inf-only 'driver' which matches all windows > > > versions; it's just one copy at the sub-toplevel directory, so it > > > won't pass the virtio_iso_path_matches_guest_os() criteria > > > > > > - there are a lot of duplicates between files for different windows > > > flavors It's due to historical reasons mostly. The best way would be having a set of separate distribution images packaged on per-platform base. Vadim. > > > > > > To sum up, the packaging and naming policy of the virtio-win rpm and the > > > virtio-win iso therein are different and neither is clear. Hardcoding > > > the policy in v2v without actually knowing it appears risky at best. > > > > > > We need guidance here from someone who knows how that virtio-win stuff > > > is packaged and how different it is across distros. Any idea whom to > > > contact on the matter? > > > > Amnon -- CC'd -- I guess? > > > > I'm aware that there are at least some differences in paths, and the > > current virt-v2v code should be able to cope for the two drivers that > > we really care about - viostor and virtio-net. Of course it can't > > install drivers that don't exist on either the ISO or the rpm. > > > > Rich. > > > > > > Anyhow, can you fix up the test at least. > > > > > > Sure, but I'd like first to figure out how to fix up the code ;) > > > > > > Roman. > > > > -- > > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > > Read my programming and virtualization blog: http://rwmj.wordpress.com > > virt-df lists disk usage of guests without needing to install any > > software inside the virtual machine. Supports Linux and Windows. > > http://people.redhat.com/~rjones/virt-df/ > >

8 41

[PATCH] daemon: always provide stdin when running chroot commands (RHBZ#1280029)
by Pino Toscano 02 Dec '15

02 Dec '15

When running commands in the mounted guest (using the "command" API, and APIs based on it), provide the /dev/null from the appliance as open fd for stdin. Commands usually assume stdin is open if they didn't close it explicitly, so this should avoid crashes or misbehavings due to that. --- daemon/command.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/daemon/command.c b/daemon/command.c index 1593de9..b2d84ca 100644 --- a/daemon/command.c +++ b/daemon/command.c @@ -23,6 +23,8 @@ #include <stdbool.h> #include <string.h> #include <sys/stat.h> +#include <sys/types.h> +#include <fcntl.h> #include "guestfs_protocol.h" #include "daemon.h" @@ -242,7 +244,7 @@ do_command (char *const *argv) { char *out; CLEANUP_FREE char *err = NULL; - int r; + int r, fd, flags; CLEANUP_BIND_STATE struct bind_state bind_state = { .mounted = false }; CLEANUP_RESOLVER_STATE struct resolver_state resolver_state = { .mounted = false }; @@ -259,6 +261,17 @@ do_command (char *const *argv) return NULL; } + /* Provide /dev/null as stdin for the command, since we want + * to make sure processes have an open stdin, and it is not + * possible to rely on the guest to provide it (Linux guests + * get /dev dynamically populated at runtime by udev). + */ + fd = open ("/dev/null", O_RDONLY|O_CLOEXEC); + if (fd == -1) { + reply_with_perror ("/dev/null"); + return NULL; + } + if (bind_mount (&bind_state) == -1) return NULL; if (enable_network) { @@ -266,8 +279,10 @@ do_command (char *const *argv) return NULL; } + flags = COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN | fd; + CHROOT_IN; - r = commandv (&out, &err, (const char * const *) argv); + r = commandvf (&out, &err, flags, (const char * const *) argv); CHROOT_OUT; free_bind_state (&bind_state); -- 2.1.0

3 10

[PATCH 1/2] resize: Work around regression in sfdisk (RHBZ#1285847).
by Richard W.M. Jones 27 Nov '15

27 Nov '15

'sfdisk --part-type' used to merely set the MBR ID byte in the partition. However since sfdisk was rewritten, it now "helpfully" corrupts the first sector of the partition if you change the type byte from an ordinary partition to an extended partition. So we need to change the order in which we sets the partition type byte, to do it before copying the partition content. This fixes a major regression in the way that virt-resize handles any guest with logical partitions (specifically Ubuntu guests). --- resize/resize.ml | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/resize/resize.ml b/resize/resize.ml index ff10fca..5ca2f20 100644 --- a/resize/resize.ml +++ b/resize/resize.ml @@ -1159,6 +1159,28 @@ read the man page virt-resize(1). g#part_add "/dev/sdb" "primary" p.p_target_start p.p_target_end ) partitions; + (* Set bootable and MBR IDs. Do this *before* copying over the data, + * because the rewritten sfdisk "helpfully" overwrites the partition + * table in the first sector of an extended partition if a partition + * is changed from primary to extended. Thus we need to set the + * MBR ID before doing the copy so sfdisk doesn't corrupt things. + *) + let set_partition_bootable_and_id p = + if p.p_bootable then + g#part_set_bootable "/dev/sdb" p.p_target_partnum true; + + may (g#part_set_name "/dev/sdb" p.p_target_partnum) p.p_label; + may (g#part_set_gpt_guid "/dev/sdb" p.p_target_partnum) p.p_guid; + + match parttype, p.p_id with + | GPT, GPT_Type gpt_type -> + g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type + | MBR, MBR_ID mbr_id -> + g#part_set_mbr_id "/dev/sdb" p.p_target_partnum mbr_id + | GPT, (No_ID|MBR_ID _) | MBR, (No_ID|GPT_Type _) -> () + in + List.iter set_partition_bootable_and_id partitions; + (* Copy over the data. *) let copy_partition p = match p.p_operation with @@ -1202,26 +1224,6 @@ read the man page virt-resize(1). in List.iter copy_partition partitions; - (* Set bootable and MBR IDs. Do this *after* copying over the data, - * so that we can magically change the primary partition to an extended - * partition if necessary. - *) - let set_partition_bootable_and_id p = - if p.p_bootable then - g#part_set_bootable "/dev/sdb" p.p_target_partnum true; - - may (g#part_set_name "/dev/sdb" p.p_target_partnum) p.p_label; - may (g#part_set_gpt_guid "/dev/sdb" p.p_target_partnum) p.p_guid; - - match parttype, p.p_id with - | GPT, GPT_Type gpt_type -> - g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type - | MBR, MBR_ID mbr_id -> - g#part_set_mbr_id "/dev/sdb" p.p_target_partnum mbr_id - | GPT, (No_ID|MBR_ID _) | MBR, (No_ID|GPT_Type _) -> () - in - List.iter set_partition_bootable_and_id partitions; - (* Fix the bootloader if we aligned the first partition. *) if align_first_partition_and_fix_bootloader then ( (* See can_fix_boot_loader above. *) -- 2.5.0

1 1

[PATCH] New API: part_expand_gpt.
by Maxim Perevedentsev 25 Nov '15

25 Nov '15

This action moves second(backup) GPT header to the end of the disk. It is usable in in-place image expanding, since free space after second GPT header is unusable. To use additional space, we have to move second header. This is what sgdisk -e does. However, sgdisk -e may perform additional actions if the partition table has unexpected params (e.g. if we call sgdisk -e /dev/sda1, it may fix partition table thus destroying the filesystem). To prevent such cases, we do a dry-run at first and fail if additional actions are scheduled. --- daemon/parted.c | 33 +++++++++++++++++++++++++++++++++ generator/actions.ml | 14 ++++++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/daemon/parted.c b/daemon/parted.c index df6b7e7..033c136 100644 --- a/daemon/parted.c +++ b/daemon/parted.c @@ -928,3 +928,36 @@ do_part_get_mbr_part_type (const char *device, int partnum) reply_with_error ("strdup failed"); return NULL; } + +int +do_part_expand_gpt(const char *device) +{ + CLEANUP_FREE char *err = NULL; + + /* If something is broken, sgdisk may try to correct it. + * (e.g. recreate partition table and so on). + * We do not want such behavior, so dry-run at first.*/ + int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, + str_sgdisk, "--pretend", "-e", device, NULL); + + if (r == -1) { + reply_with_error ("%s --pretend -e %s: %s", str_sgdisk, device, err); + return -1; + } + if (err && strlen(err) != 0) { + /* Unexpected actions. */ + reply_with_error ("%s --pretend -e %s: %s", str_sgdisk, device, err); + return -1; + } + + /* Now we can do a real run. */ + r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, + str_sgdisk, "-e", device, NULL); + + if (r == -1) { + reply_with_error ("%s -e %s: %s", str_sgdisk, device, err); + return -1; + } + + return 0; +} diff --git a/generator/actions.ml b/generator/actions.ml index 7ab8ee4..3a14683 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12787,6 +12787,20 @@ See also L<ntfsresize(8)>, L<resize2fs(8)>, L<btrfs(8)>, L<xfs_info(8)>." }; longdesc = "\ This is the internal call which implements C<guestfs_feature_available>." }; + { defaults with + name = "part_expand_gpt"; added = (1, 31, 29); + style = RErr, [Device "device"], []; + proc_nr = Some 459; + optional = Some "gdisk"; + shortdesc = "move backup GPT header to the end of the disk"; + longdesc = "\ +Move backup GPT data structures to the end of the disk. +This is useful in case of in-place image expand +since disk space after backup GPT header is not usable. +This is equivalent to C<sgdisk -e>. + +See also L<sgdisk(8)>." }; + ] (* Non-API meta-commands available only in guestfish. diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c92ddb6..658bd78 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -458 +459 -- 1.8.3.1

1 1

[PATCH] fuse: fix return value of guestunmount for unmounted paths
by Pino Toscano 24 Nov '15

24 Nov '15

Exit with 3 as return value when fusermount fails, because the specified mount point is not considered mounted for the user. This is in line with what the guestunmount documentation says. Adapt the test-guestunmount-fd test to the updated return value. Thanks to: Maxim Perevedentsev. --- fuse/guestunmount.c | 2 +- fuse/test-guestunmount-fd.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fuse/guestunmount.c b/fuse/guestunmount.c index e952c63..fa6d5bb 100644 --- a/fuse/guestunmount.c +++ b/fuse/guestunmount.c @@ -220,7 +220,7 @@ main (int argc, char *argv[]) free (error); - exit (2); + exit (3); /* success */ done: diff --git a/fuse/test-guestunmount-fd.c b/fuse/test-guestunmount-fd.c index 93adc98..b09a60f 100644 --- a/fuse/test-guestunmount-fd.c +++ b/fuse/test-guestunmount-fd.c @@ -107,8 +107,8 @@ main (int argc, char *argv[]) } /* Close the write side of the pipe. This should cause guestunmount - * to exit. It should exit with status code _2_ because we gave it - * a mountpoint which isn't a FUSE mountpoint. + * to exit. It should exit with status code _3_ because we gave it + * a directory which isn't a FUSE mountpoint. */ close (pipefd[1]); @@ -117,10 +117,10 @@ main (int argc, char *argv[]) perror ("waitpid"); exit (EXIT_FAILURE); } - if (!WIFEXITED (status) || WEXITSTATUS (status) != 2) { + if (!WIFEXITED (status) || WEXITSTATUS (status) != 3) { char status_string[80]; - fprintf (stderr, "%s: test failed: guestunmount didn't return status code 2; %s\n", + fprintf (stderr, "%s: test failed: guestunmount didn't return status code 3; %s\n", guestfs_int_program_name, guestfs_int_exit_status_to_string (status, "guestunmount", status_string, -- 2.1.0

2 1

Any risk in sparsifying a base image (that has a snapshot on top of it)
by Yaniv Kaul 24 Nov '15

24 Nov '15

Assuming the VM is not running, and we have a base (raw, sparse) with a snapshot (qcow2) on top of it. Is there any issue with running virt-sparsify on the base image? I assume deleted blocks in the base can be sparsified, since they are either still deleted on the snap (which is fine) or were written in the snap (which is fine either and does not change or matter for the base image). Can I assume sparsifying should not make any changes that matter for a VM that will be spun (post the process)? TIA, Y.

2 2

installation error
by Keresztes Péter-Zoltán 24 Nov '15

24 Nov '15

Hello, I am trying to install libguestfs on a proxmox (debian jessie with some custom packages) and when I run make at a certain point I get this error: E: Can't find a source to download version '2.02.116-pve1' of 'lvm2:amd64' supermin: cd '/tmp/supermin3f8d1d.tmpdir/pzaggirn' && /usr/bin/apt-get download 'bsdmainutils' 'iproute2' 'isc-dhcp-client' 'libc-bin' 'openssh-client' 'systemd' 'vim-tiny' 'bash' 'e2fsprogs' 'kmod' 'lvm2' 'procps' 'rsync' 'tar' 'udev' 'util-linux' 'libc6' 'libmagic1' 'grub-common' 'dpkg' 'fuse' 'libgssapi-krb5-2' 'adduser' 'passwd' 'initscripts' 'sysv-rc' 'vim-common' 'base-files' 'debconf' 'libpam-modules' 'libaudit-common' 'insserv' 'startpar' 'libldap-2.4-2' 'libsemanage-common' 'perl-modules': command failed, see earlier errors Makefile:1891: recipe for target 'stamp-supermin' failed make[2]: *** [stamp-supermin] Error 1 make[2]: Leaving directory '/root/libguestfs-1.30.4/appliance' Makefile:1923: recipe for target 'all-recursive' failed make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory '/root/libguestfs-1.30.4' Makefile:1795: recipe for target 'all' failed make: *** [all] Error 2 Anyone has a solution for this issue? Regards Peter

2 5