On Fri, Dec 13, 2013 at 04:32:49PM +0100, Pino Toscano wrote:
> The current add_cdrom way basically appends a new raw "-cdrom /path"
> parameter to the qemu invocation (even when using libvirt as backend),
> hence such images are seen as "CD-ROM drives" inside the appliance.
> However, there is no need for such particular behaviour, as they need to
> be handled as normal (read-only) drives.
>
> Adding CD-ROM disk images as drives also changes the device names used
> for them inside the appliance from /dev/srN to the usual e.g. /dev/sdX.
>
> These changes fix different issues:
> - it is possible to start guestfish without adding disks with -a, then
> just add-cdrom and run
> - list-devices does not cause guestfishd to crash when sorting the list
s/guestfishd/guestfsd/
> of devices (exposed by the test case in RHBZ#563450)
> - the result of list-devices now reflects the order images were added
> (RHBZ#563450)
>
> Add two small regression tests for the fixes described above.
> ---
> src/drives.c | 7 +-----
> tests/regressions/Makefile.am | 2 ++
> tests/regressions/rhbz563450.sh | 54
> ++++++++++++++++++++++++++++++++++++++++
> tests/regressions/rhbz563450b.sh | 43 ++++++++++++++++++++++++++++++++
> 4 files changed, 100 insertions(+), 6 deletions(-)
> create mode 100755 tests/regressions/rhbz563450.sh
> create mode 100755 tests/regressions/rhbz563450b.sh
>
> diff --git a/src/drives.c b/src/drives.c
> index 16798a3..4b65dda 100644
> --- a/src/drives.c
> +++ b/src/drives.c
> @@ -1087,12 +1087,7 @@ guestfs__add_cdrom (guestfs_h *g, const char *filename)
> return -1;
> }
>
> - if (access (filename, F_OK) == -1) {
> - perrorf (g, "%s", filename);
> - return -1;
> - }
> -
> - return guestfs_config (g, "-cdrom", filename);
> + return guestfs__add_drive_ro (g, filename);
> }
I don't think this implementation is quite right because it leaves the
[now] bogus check for ':' in the filename. guestfs_add_drive_ro can
handle ':' in the filename (because it creates an overlay):
$ truncate -s 100M foo:bar
$ guestfish --ro -a foo:bar
Welcome to guestfish, the guest filesystem shell for
editing virtual machine filesystems and disk images.
Type: 'help' for help on commands
'man' to read the manual
'quit' to quit the shell
><fs> run
I think it's better to remove the contents of the guestfs__add_cdrom
function completely, so the function will become just:
int
guestfs__add_cdrom (guestfs_h *g, const char *filename)
{
return guestfs__add_drive_ro (g, filename);
}
[...]
> +# https://bugzilla.redhat.com/show_bug.cgi?id=563450
> +# Test the order of added images
> +
> +set -e
> +export LANG=C
> +
> +rm -f test.out
> +
> +../../fish/guestfish --ro > test.out <<EOF
> +add-drive-ro ../guests/fedora.img
> +add-cdrom ../data/test.iso
> +add-drive-ro ../guests/debian.img
Earlier in the test, you'll probably need to add this:
if [ ! -s ../guests/fedora.img -o ! -s ../guests/debian.img ]; then
echo "$0: test skipped because there is no fedora.img or debian.img"
exit 77
fi
---
Also, should we update the documentation?
http://libguestfs.org/guestfs.3.html#guestfs_add_cdrom
Maybe or maybe not worth it.
---
Patch looks good apart from these three small issues.
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#)
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
