[PATCH 0/3] kvmtool: don't overwrite /virt/init

[PATCH 0/3] kvmtool: don't overwrite /virt/init

[Oleg Nesterov]

Hello,

On top of "[PATCH 0/3] kvmtool: tiny init fox x86_64" I sent.

I simply can't understand why kvmtool always overwrites /virt/init.
This doesn't look right, especially because you can't pass "init="
kernel argument without another patch "[PATCH] kvmtool/run: append
cfg.kernel_cmdline at the end of real_cmdline" I sent. And so far
it is not clear to me if you are going to accept that patch or not.

1/3 and 2/3 are off-topic cleanups.

Oleg.

[PATCH 1/3] kvmtool: add lkvm-static to gitignore

[Oleg Nesterov]

add lkvm-static to gitignore

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 697a63f..a16a97f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 /lkvm
+/lkvm-static
 /vm
 *.o
 *.d
-- 
2.4.3

[PATCH 2/3] kvmtool/run: don't abuse "root=" parameter, don't pass "rw" to v9fs_mount()

[Oleg Nesterov]

1. kvm_cmd_run_init() appends "root=/dev/root" to real_cmdline if
   cfg.using_rootfs == T. This doesn't hurt but makes no sense and
   looks confusing.

   We do not need to initialiaze the kernel's saved_root_name[] and
   "/dev/root" means nothing to name_to_dev_t().

   We only need to pass this mount-tag to 9p but the kernel always
   uses dev_name="/dev/root" in mount_root() path, so we can safely
   remove this option from the command line.

2. "rw" in rootflags looks confusing too, it is silently ignored by
   v9fs_parse_options() and has no effect.

   We need to clear MS_RDONLY from root_mountflags, this is what the
   "standalone" kernel parameter correctly does.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 builtin-run.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin-run.c b/builtin-run.c
index ab1fc2a..6e4491c 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -590,7 +590,7 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 	}
 
 	if (kvm->cfg.using_rootfs) {
-		strcat(real_cmdline, " root=/dev/root rw rootflags=rw,trans=virtio,version=9p2000.L rootfstype=9p");
+		strcat(real_cmdline, " rw rootflags=trans=virtio,version=9p2000.L rootfstype=9p");
 		if (kvm->cfg.custom_rootfs) {
 			kvm_run_set_sandbox(kvm);
 
-- 
2.4.3

[PATCH 3/3] kvmtool/run: do not overwrite /virt/init

[Oleg Nesterov]

To me kvm_setup_guest_init() behaviour looks "obviously wrong" and
unfriendly because it always overwrites /virt/init.

kvm_setup_guest_init() is also called when we are going to use this
tree as a rootfs, and without another patch ("kvmtool/run: append
cfg.kernel_cmdline at the end of real_cmdline") the user can't use
"lkvm run -p init=my_init_path". This simply means that you can not
use a customized init unless you patch kvmtool.

Change extract_file() to do nothing if the file already exists. This
should not affect do_setup() which calls kvm_setup_guest_init() only
if make_dir(guestfs_name) creates the new/empty dir.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 builtin-setup.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin-setup.c b/builtin-setup.c
index 1e5b1e4..8be8d62 100644
--- a/builtin-setup.c
+++ b/builtin-setup.c
@@ -130,10 +130,14 @@ static int extract_file(const char *guestfs_name, const char *filename,
 
 	snprintf(path, PATH_MAX, "%s%s/%s", kvm__get_dir(),
 				guestfs_name, filename);
-	remove(path);
-	fd = open(path, O_CREAT | O_WRONLY, 0755);
-	if (fd < 0)
+
+	fd = open(path, O_EXCL | O_CREAT | O_WRONLY, 0755);
+	if (fd < 0) {
+		if (errno == EEXIST)
+			return 0;
 		die("Fail to setup %s", path);
+	}
+
 	ret = xwrite(fd, data, (size_t)_size);
 	if (ret < 0)
 		die("Fail to setup %s", path);
-- 
2.4.3

Re: [PATCH 0/3] kvmtool: don't overwrite /virt/init

[Will Deacon]

Hi Oleg,

On Thu, Oct 22, 2015 at 05:59:21PM +0200, Oleg Nesterov wrote:
> On top of "[PATCH 0/3] kvmtool: tiny init fox x86_64" I sent.
> 
> I simply can't understand why kvmtool always overwrites /virt/init.
> This doesn't look right, especially because you can't pass "init="
> kernel argument without another patch "[PATCH] kvmtool/run: append
> cfg.kernel_cmdline at the end of real_cmdline" I sent. And so far
> it is not clear to me if you are going to accept that patch or not.

Sorry, I was off last week and didn't have email access. I'm going through
my inbox now and your patches look pretty good to me. I'll apply/test and
then push out if I don't see any issues.

Thanks!

Will