Re: [PATCH v2 2/2] kvm tools: Add '--rootfs' and '--binsh'

[Pekka Enberg <penberg@xxxxxxxxxx>]

On 8/3/11 5:52 AM, Aneesh Kumar K.V wrote:
> On Tue,  2 Aug 2011 21:46:29 +0300, Sasha Levin<levinsasha928@xxxxxxxxx>  wrote:
> > This patch adds 2 new flags:
> >
> >   --rootfs [path] - Specifies a path to use as rootfs. The path will be mounted
> > using virtio-9p and booted from.
> > Easiest way to test it is to mount the sample image we recommend with kvm tool
> > (http://wiki.qemu.org/download/linux-0.2.img.bz2) and mounting it somewhere.
> > *PLEASE DO NOT TRY BOOTING FROM YOUR REAL ROOT AKA '/', EVEN WITH READ ONLY
> > SET* - Lets make sure the code is bulletproof before we delete your rootfs :)
> >
> >   --binsh - This flag will boot into simple /bin/sh shell instead of going
> > through /sbin/init. This is usefull for more complex distribution rootfs
> > where we have to deal with /etc/fstab and such.
> >
> > Signed-off-by: Sasha Levin<levinsasha928@xxxxxxxxx>
> Reviewed-by: Aneesh Kumar K.V<aneesh.kumar@xxxxxxxxxxxxxxxxxx>
>
> > ---
> >   tools/kvm/builtin-run.c |   18 +++++++++++++++++-
> >   1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> > index 2e04265..78680ec 100644
> > --- a/tools/kvm/builtin-run.c
> > +++ b/tools/kvm/builtin-run.c
> > @@ -76,11 +76,13 @@ static const char *guest_mac;
> >   static const char *host_mac;
> >   static const char *script;
> >   static const char *guest_name;
> > +static const char *rootfs;
> >   static bool single_step;
> >   static bool readonly_image[MAX_DISK_IMAGES];
> >   static bool vnc;
> >   static bool sdl;
> >   static bool balloon;
> > +static bool bin_sh;
> >   extern bool ioport_debug;
> >   extern int  active_console;
> >   extern int  debug_iodelay;
> > @@ -156,6 +158,8 @@ static const struct option options[] = {
> >   	OPT_BOOLEAN('\0', "balloon",&balloon, "Enable virtio balloon"),
> >   	OPT_BOOLEAN('\0', "vnc",&vnc, "Enable VNC framebuffer"),
> >   	OPT_BOOLEAN('\0', "sdl",&sdl, "Enable SDL framebuffer"),
> > +	OPT_STRING('\0', "rootfs",&rootfs, "path", "Use path as rootfs"),
> > +	OPT_BOOLEAN('\0', "binsh",&bin_sh, "Boot rootfs to /bin/sh instead of /sbin/init"),
> >
> >   	OPT_GROUP("Kernel options:"),
> >   	OPT_STRING('k', "kernel",&kernel_filename, "kernel",
> > @@ -615,7 +619,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
> >   		strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
> >
> >   	hi = NULL;
> > -	if (!image_filename[0]) {
> > +	if (!rootfs&&  !image_filename[0]) {
> >   		hi = host_image(real_cmdline, sizeof(real_cmdline));
> >   		if (hi) {
> >   			image_filename[0] = hi;
> > @@ -627,6 +631,18 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
> >   	if (!strstr(real_cmdline, "root="))
> >   		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
> >
> > +	if (rootfs) {
> > +		char tmp[PATH_MAX];
> > +
> > +		if (realpath(rootfs, tmp) == 0 ||
> > +		    virtio_9p__init(kvm, tmp, "/dev/root")<  0)
> > +			die("Unable to initialize virtio 9p");
> > +		strcat(real_cmdline, " root=/dev/root rootflags=rw,trans=virtio,version=9p2000.u rootfstype=9p");
> > +
> > +		if (bin_sh)
> > +			strcat(real_cmdline, " init=/bin/sh");
> > +	}

Why don't we use the "-d" command line option for this? We can
use fstat() and S_ISDIR to detect that the user specified a directory
as a disk image?

I'm also not completely convinced we should introduce the 'binsh'
command line option. Instead, I'd simply default to 'init=/bin/sh'
unless the user uses -p to specify init.

                        Pekka
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html