kvmtool: don't overwrite /virt/init

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.

add lkvm-static to gitignore

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

index 697a63f..a16a97f 100644
@@ -1,4 +1,5 @@
 /lkvm
+/lkvm-static
 /vm
 *.o
 *.d

2.4.3

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.

---
 builtin-run.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index ab1fc2a..6e4491c 100644
@@ -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

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.

---
 builtin-setup.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

index 1e5b1e4..8be8d62 100644
@@ -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

Hi Oleg,

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