From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pekka Enberg Subject: Re: [PATCH v2 2/2] kvm tools: Add '--rootfs' and '--binsh' Date: Wed, 03 Aug 2011 16:14:18 +0300 Message-ID: <4E3949AA.6070602@kernel.org> References: <1312310789-22817-1-git-send-email-levinsasha928@gmail.com> <1312310789-22817-2-git-send-email-levinsasha928@gmail.com> <874o1znr46.fsf@skywalker.in.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Sasha Levin , kvm@vger.kernel.org, mingo@elte.hu, asias.hejun@gmail.com, gorcunov@gmail.com To: "Aneesh Kumar K.V" Return-path: Received: from mail-ww0-f44.google.com ([74.125.82.44]:62699 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755716Ab1HCNOY (ORCPT ); Wed, 3 Aug 2011 09:14:24 -0400 Received: by wwe5 with SMTP id 5so822576wwe.1 for ; Wed, 03 Aug 2011 06:14:22 -0700 (PDT) In-Reply-To: <874o1znr46.fsf@skywalker.in.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 8/3/11 5:52 AM, Aneesh Kumar K.V wrote: > On Tue, 2 Aug 2011 21:46:29 +0300, Sasha Levin 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 > Reviewed-by: Aneesh Kumar K.V > >> --- >> 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