From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IJwxY-0006bT-0M for qemu-devel@nongnu.org; Sat, 11 Aug 2007 15:49:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IJwxW-0006Zb-E0 for qemu-devel@nongnu.org; Sat, 11 Aug 2007 15:49:14 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IJwxW-0006ZU-AB for qemu-devel@nongnu.org; Sat, 11 Aug 2007 15:49:14 -0400 Received: from wx-out-0506.google.com ([66.249.82.225]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IJwxV-0002mj-Ey for qemu-devel@nongnu.org; Sat, 11 Aug 2007 15:49:14 -0400 Received: by wx-out-0506.google.com with SMTP id h31so846105wxd for ; Sat, 11 Aug 2007 12:49:12 -0700 (PDT) Message-ID: <46BE12B9.1000203@codemonkey.ws> Date: Sat, 11 Aug 2007 14:49:13 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images References: <59abf66e0708081124g14901b01i841b70d17ae1e097@mail.gmail.com> <59abf66e0708081252of2948d7we85c9084bad245d4@mail.gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, "Daniel P. Berrange" andrzej zaborowski wrote: > On 08/08/07, Jorge Lucángeli Obes wrote: > >> This patch makes QEMU check for command line options stored in qcow2 images. >> >> Signed-off-by: Laurent Vivier >> Signed-off-by: Jorge Lucángeli Obes >> --- >> diff --git a/qemu/vl.c b/qemu/vl.c >> index 4ad39f1..1d28794 100644 >> --- a/qemu/vl.c >> +++ b/qemu/vl.c >> @@ -184,6 +184,11 @@ int time_drift_fix = 0; >> const char *cpu_vendor_string; >> >> /***********************************************************/ >> +/* Image annotation support */ >> + >> +char *img_get_annot(char *filename); >> + >> +/***********************************************************/ >> /* x86 ISA bus support */ >> >> target_phys_addr_t isa_mem_base = 0; >> @@ -6917,6 +6922,13 @@ int main(int argc, char **argv) >> char usb_devices[MAX_USB_CMDLINE][128]; >> int usb_devices_index; >> int fds[2]; >> + char *tmpannot; >> + char annot[1024]; >> + int done = 0; >> + unsigned int nbtoks = 0; >> + char *tok; >> + BlockDriver *drv; >> + BlockDriverState *bs; >> >> saved_argc = argc; >> saved_argv = argv; >> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv) >> nb_nics = 0; >> /* default mac address of the first network interface */ >> >> + bdrv_init(); >> + >> + drv = bdrv_find_format("qcow2"); >> + >> + if (argc > 1 && argv[1][0] != '-') { >> + bs = bdrv_new(""); >> + if (!bs) { >> + fprintf(stderr, "Not enough memory"); >> + exit(1); >> + } >> + if (bdrv_open2(bs, argv[1], 0, drv) < 0) { >> + fprintf(stderr, "Could not open '%s'", argv[1]); >> + bdrv_delete(bs); >> + exit(1); >> + } >> + >> + tmpannot = bdrv_get_annot(bs, "commandline_args"); >> + if (tmpannot) { >> + pstrcpy(annot, 1024, tmpannot); >> > > I thought one of the arguments for storing config options was the > commandline length limit, which is 32k or so characters on most > systems. > > >> + >> + do { >> + tok = strtok(nbtoks == 0? tmpannot : NULL, " "); >> + >> + if (tok != NULL) >> + nbtoks++; >> + else >> + done = 1; >> + } while (!done); >> + >> + free(tmpannot); >> + >> + if (nbtoks > 0) { >> + char **argvprime = malloc((nbtoks + argc) * sizeof(char*)); >> + >> + for (i = 0; i < argc; i++) >> + argvprime[i] = argv[i]; >> + >> + for (i = 0; i < nbtoks; i++) >> + argvprime[i + argc] = strtok(i == 0? annot : NULL, " "); >> + >> + argv = argvprime; >> + argc = argc + nbtoks; >> + >> + for (i = 0; i < nbtoks + 2; i++) >> + printf("argv[%d] = %s\n", i, argv[i]); >> + >> + } >> + } >> + >> + bdrv_delete(bs); >> + } >> + >> optind = 1; >> for(;;) { >> if (optind >= argc) >> @@ -7558,7 +7622,6 @@ int main(int argc, char **argv) >> #endif >> >> /* we always create the cdrom drive, even if no disk is there */ >> - bdrv_init(); >> if (cdrom_index >= 0) { >> bs_table[cdrom_index] = bdrv_new("cdrom"); >> bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM); >> @@ -7764,5 +7827,8 @@ int main(int argc, char **argv) >> >> main_loop(); >> quit_timers(); >> + >> + /* was reassigned above so it needs free()ing */ >> + free(argv); >> return 0; >> } >> >> >> > > Yes, the file format starting with "#! /path/to/qemu" is a much better > idea, but the commandline length limitation has to be taken care of > separately, perhaps the switches should follow on next lines in the > file while the #! line shouldn't specify the switches. > Yes, I think that's a very good idea. I think it should be extended slightly to incorporate Dan's original suggestion of using an option to specify that the command line ought to be read from the image. So you'd end up with: #!/path/to/qemu -read-args-from-image -m 512 -net tap -net nic,model=rtl8139 Such that this whole section was sector aligned. The "#!" is the magic for the disk image type. On windows, you could completely omit /path/to/qemu and just set up an extension association that would associate .qemu with "/path/to/qemu -read-args-from-image %s" and then it would work equally well with Windows. Regards, Anthony Liguori > Regards, > Andrzej (away until mid-September(?)) >