* [Qemu-devel] [PATCH] allow bootdevice change from the monitor
@ 2008-03-26 13:12 Gildas
2008-03-29 0:29 ` Aurelien Jarno
0 siblings, 1 reply; 4+ messages in thread
From: Gildas @ 2008-03-26 13:12 UTC (permalink / raw)
To: qemu-devel
This patch allows changing the boot device from within the monitor.
Signed-off-by: Gildas Le Nadan <3ntr0p13@gmail.com>
diff -Nur a/hw/pc.c b/hw/pc.c
--- a/hw/pc.c 2008-03-18 07:53:05.000000000 +0100
+++ b/hw/pc.c 2008-03-26 13:18:16.000000000 +0100
@@ -180,6 +180,33 @@
return 0;
}
+/* copy/pasted from cmos_init, should be made a general function
+ and used there as well */
+int pc_set_bootdevice(const char *boot_device)
+{
+#define PC_MAX_BOOT_DEVICES 3
+ RTCState *s = rtc_state;
+ int nbds, bds[3] = { 0, };
+ int i;
+
+ nbds = strlen(boot_device);
+ if (nbds > PC_MAX_BOOT_DEVICES) {
+ fprintf(stderr, "Too many boot devices for PC\n");
+ return(1);
+ }
+ for (i = 0; i < nbds; i++) {
+ bds[i] = boot_device2nibble(boot_device[i]);
+ if (bds[i] == 0) {
+ fprintf(stderr, "Invalid boot device for PC: '%c'\n",
+ boot_device[i]);
+ return(1);
+ }
+ }
+ rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
+ rtc_set_memory(s, 0x38, (bds[2] << 4));
+ return(0);
+}
+
/* hd_table must contain 4 block drivers */
static void cmos_init(int ram_size, const char *boot_device,
BlockDriverState **hd_table)
{
@@ -717,6 +744,8 @@
BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
BlockDriverState *fd[MAX_FD];
+ set_bootdevice_fct = pc_set_bootdevice;
+
linux_boot = (kernel_filename != NULL);
/* init CPUs */
diff -Nur a/monitor.c b/monitor.c
--- a/monitor.c 2008-02-10 17:33:13.000000000 +0100
+++ b/monitor.c 2008-03-26 13:17:56.000000000 +0100
@@ -993,6 +993,21 @@
suffix, addr, size * 2, val);
}
+static void do_bootdevice_set(const char *bootdevice)
+{
+ int res;
+
+ if (set_bootdevice_fct != NULL) {
+ res = set_bootdevice_fct(bootdevice);
+ if (res == 0)
+ term_printf("boot device list now set to %s\n", bootdevice);
+ else
+ term_printf("setting boot device list failed with error
%i\n", res);
+ } else {
+ term_printf("no function defined to set boot device list for
this architecture\n");
+ }
+}
+
static void do_system_reset(void)
{
qemu_system_reset_request();
@@ -1328,6 +1343,8 @@
"capture index", "stop capture" },
{ "memsave", "lis", do_memory_save,
"addr size file", "save to disk virtual memory dump starting at
'addr' of size 'size'", },
+ { "boot_set", "s", do_bootdevice_set,
+ "bootdevice", "define new values for the boot device list" },
{ NULL, NULL, },
};
diff -Nur a/sysemu.h b/sysemu.h
--- a/sysemu.h 2008-03-18 07:53:05.000000000 +0100
+++ b/sysemu.h 2008-03-26 13:12:20.000000000 +0100
@@ -9,6 +9,11 @@
extern int vm_running;
extern const char *qemu_name;
+/* handler to set the boot_device for a specific type of QEMUMachine */
+/* return 0 if success */
+typedef int QEMUMachineSetBootFunc(const char *boot_device);
+QEMUMachineSetBootFunc *set_bootdevice_fct;
+
typedef struct vm_change_state_entry VMChangeStateEntry;
typedef void VMChangeStateHandler(void *opaque, int running);
typedef void VMStopHandler(void *opaque, int reason);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] allow bootdevice change from the monitor
2008-03-26 13:12 [Qemu-devel] [PATCH] allow bootdevice change from the monitor Gildas
@ 2008-03-29 0:29 ` Aurelien Jarno
2008-03-30 7:32 ` Gildas
0 siblings, 1 reply; 4+ messages in thread
From: Aurelien Jarno @ 2008-03-29 0:29 UTC (permalink / raw)
To: qemu-devel
On Wed, Mar 26, 2008 at 02:12:25PM +0100, Gildas wrote:
> This patch allows changing the boot device from within the monitor.
I have just updated BOCHS BIOS from upstream CVS. The new version allows
to select the boot device via a menu after the POST. I am therefore not
sure anymore that your patch is still useful. What do you think?
> Signed-off-by: Gildas Le Nadan <3ntr0p13@gmail.com>
>
> diff -Nur a/hw/pc.c b/hw/pc.c
> --- a/hw/pc.c 2008-03-18 07:53:05.000000000 +0100
> +++ b/hw/pc.c 2008-03-26 13:18:16.000000000 +0100
> @@ -180,6 +180,33 @@
> return 0;
> }
>
> +/* copy/pasted from cmos_init, should be made a general function
> + and used there as well */
> +int pc_set_bootdevice(const char *boot_device)
> +{
> +#define PC_MAX_BOOT_DEVICES 3
> + RTCState *s = rtc_state;
> + int nbds, bds[3] = { 0, };
> + int i;
> +
> + nbds = strlen(boot_device);
> + if (nbds > PC_MAX_BOOT_DEVICES) {
> + fprintf(stderr, "Too many boot devices for PC\n");
> + return(1);
> + }
> + for (i = 0; i < nbds; i++) {
> + bds[i] = boot_device2nibble(boot_device[i]);
> + if (bds[i] == 0) {
> + fprintf(stderr, "Invalid boot device for PC: '%c'\n",
> + boot_device[i]);
> + return(1);
> + }
> + }
> + rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
> + rtc_set_memory(s, 0x38, (bds[2] << 4));
> + return(0);
> +}
> +
> /* hd_table must contain 4 block drivers */
> static void cmos_init(int ram_size, const char *boot_device,
> BlockDriverState **hd_table)
> {
> @@ -717,6 +744,8 @@
> BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> BlockDriverState *fd[MAX_FD];
>
> + set_bootdevice_fct = pc_set_bootdevice;
> +
> linux_boot = (kernel_filename != NULL);
>
> /* init CPUs */
> diff -Nur a/monitor.c b/monitor.c
> --- a/monitor.c 2008-02-10 17:33:13.000000000 +0100
> +++ b/monitor.c 2008-03-26 13:17:56.000000000 +0100
> @@ -993,6 +993,21 @@
> suffix, addr, size * 2, val);
> }
>
> +static void do_bootdevice_set(const char *bootdevice)
> +{
> + int res;
> +
> + if (set_bootdevice_fct != NULL) {
> + res = set_bootdevice_fct(bootdevice);
> + if (res == 0)
> + term_printf("boot device list now set to %s\n", bootdevice);
> + else
> + term_printf("setting boot device list failed with error
> %i\n", res);
> + } else {
> + term_printf("no function defined to set boot device list for
> this architecture\n");
> + }
> +}
> +
> static void do_system_reset(void)
> {
> qemu_system_reset_request();
> @@ -1328,6 +1343,8 @@
> "capture index", "stop capture" },
> { "memsave", "lis", do_memory_save,
> "addr size file", "save to disk virtual memory dump starting at
> 'addr' of size 'size'", },
> + { "boot_set", "s", do_bootdevice_set,
> + "bootdevice", "define new values for the boot device list" },
> { NULL, NULL, },
> };
>
> diff -Nur a/sysemu.h b/sysemu.h
> --- a/sysemu.h 2008-03-18 07:53:05.000000000 +0100
> +++ b/sysemu.h 2008-03-26 13:12:20.000000000 +0100
> @@ -9,6 +9,11 @@
> extern int vm_running;
> extern const char *qemu_name;
>
> +/* handler to set the boot_device for a specific type of QEMUMachine */
> +/* return 0 if success */
> +typedef int QEMUMachineSetBootFunc(const char *boot_device);
> +QEMUMachineSetBootFunc *set_bootdevice_fct;
> +
> typedef struct vm_change_state_entry VMChangeStateEntry;
> typedef void VMChangeStateHandler(void *opaque, int running);
> typedef void VMStopHandler(void *opaque, int reason);
>
>
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] allow bootdevice change from the monitor
2008-03-29 0:29 ` Aurelien Jarno
@ 2008-03-30 7:32 ` Gildas
2008-04-08 20:54 ` Aurelien Jarno
0 siblings, 1 reply; 4+ messages in thread
From: Gildas @ 2008-03-30 7:32 UTC (permalink / raw)
To: qemu-devel
2008/3/29, Aurelien Jarno <aurelien@aurel32.net>:
> On Wed, Mar 26, 2008 at 02:12:25PM +0100, Gildas wrote:
> > This patch allows changing the boot device from within the monitor.
>
>
> I have just updated BOCHS BIOS from upstream CVS. The new version allows
> to select the boot device via a menu after the POST. I am therefore not
> sure anymore that your patch is still useful. What do you think?
Hi Aurelien,
I see the monitor as some kind of iLO/management interface and I think
the two approaches are somewhat orthogonal and have different use
cases, so I believe they should be both present:
- the boot menu after POST is user interactive
- the command in the monitor may be interactive but can also be
triggered by scripts (think automated installation such as FAI which
boot on pxe then on the local drive) or a management layer (think
libvirt).
Also I wanted the entry in the monitor menu to be hardware independant
so even if there is only x86 support for now it might be possible to
use a single approach to set the boot device for all archs (at least
the ones making use of the bootdevice parameter, such as sparc and
ppc).
Regards,
Gildas
> > Signed-off-by: Gildas Le Nadan <3ntr0p13@gmail.com>
> >
> > diff -Nur a/hw/pc.c b/hw/pc.c
> > --- a/hw/pc.c 2008-03-18 07:53:05.000000000 +0100
> > +++ b/hw/pc.c 2008-03-26 13:18:16.000000000 +0100
> > @@ -180,6 +180,33 @@
> > return 0;
> > }
> >
> > +/* copy/pasted from cmos_init, should be made a general function
> > + and used there as well */
> > +int pc_set_bootdevice(const char *boot_device)
> > +{
> > +#define PC_MAX_BOOT_DEVICES 3
> > + RTCState *s = rtc_state;
> > + int nbds, bds[3] = { 0, };
> > + int i;
> > +
> > + nbds = strlen(boot_device);
> > + if (nbds > PC_MAX_BOOT_DEVICES) {
> > + fprintf(stderr, "Too many boot devices for PC\n");
> > + return(1);
> > + }
> > + for (i = 0; i < nbds; i++) {
> > + bds[i] = boot_device2nibble(boot_device[i]);
> > + if (bds[i] == 0) {
> > + fprintf(stderr, "Invalid boot device for PC: '%c'\n",
> > + boot_device[i]);
> > + return(1);
> > + }
> > + }
> > + rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
> > + rtc_set_memory(s, 0x38, (bds[2] << 4));
> > + return(0);
> > +}
> > +
> > /* hd_table must contain 4 block drivers */
> > static void cmos_init(int ram_size, const char *boot_device,
> > BlockDriverState **hd_table)
> > {
> > @@ -717,6 +744,8 @@
> > BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> > BlockDriverState *fd[MAX_FD];
> >
> > + set_bootdevice_fct = pc_set_bootdevice;
> > +
> > linux_boot = (kernel_filename != NULL);
> >
> > /* init CPUs */
> > diff -Nur a/monitor.c b/monitor.c
> > --- a/monitor.c 2008-02-10 17:33:13.000000000 +0100
> > +++ b/monitor.c 2008-03-26 13:17:56.000000000 +0100
> > @@ -993,6 +993,21 @@
> > suffix, addr, size * 2, val);
> > }
> >
> > +static void do_bootdevice_set(const char *bootdevice)
> > +{
> > + int res;
> > +
> > + if (set_bootdevice_fct != NULL) {
> > + res = set_bootdevice_fct(bootdevice);
> > + if (res == 0)
> > + term_printf("boot device list now set to %s\n", bootdevice);
> > + else
> > + term_printf("setting boot device list failed with error
> > %i\n", res);
> > + } else {
> > + term_printf("no function defined to set boot device list for
> > this architecture\n");
> > + }
> > +}
> > +
> > static void do_system_reset(void)
> > {
> > qemu_system_reset_request();
> > @@ -1328,6 +1343,8 @@
> > "capture index", "stop capture" },
> > { "memsave", "lis", do_memory_save,
> > "addr size file", "save to disk virtual memory dump starting at
> > 'addr' of size 'size'", },
> > + { "boot_set", "s", do_bootdevice_set,
> > + "bootdevice", "define new values for the boot device list" },
> > { NULL, NULL, },
> > };
> >
> > diff -Nur a/sysemu.h b/sysemu.h
> > --- a/sysemu.h 2008-03-18 07:53:05.000000000 +0100
> > +++ b/sysemu.h 2008-03-26 13:12:20.000000000 +0100
> > @@ -9,6 +9,11 @@
> > extern int vm_running;
> > extern const char *qemu_name;
> >
> > +/* handler to set the boot_device for a specific type of QEMUMachine */
> > +/* return 0 if success */
> > +typedef int QEMUMachineSetBootFunc(const char *boot_device);
> > +QEMUMachineSetBootFunc *set_bootdevice_fct;
> > +
> > typedef struct vm_change_state_entry VMChangeStateEntry;
> > typedef void VMChangeStateHandler(void *opaque, int running);
> > typedef void VMStopHandler(void *opaque, int reason);
> >
> >
> >
>
>
> --
> .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
> : :' : Debian developer | Electrical Engineer
> `. `' aurel32@debian.org | aurelien@aurel32.net
> `- people.debian.org/~aurel32 | www.aurel32.net
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] allow bootdevice change from the monitor
2008-03-30 7:32 ` Gildas
@ 2008-04-08 20:54 ` Aurelien Jarno
0 siblings, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2008-04-08 20:54 UTC (permalink / raw)
To: qemu-devel
On Sun, Mar 30, 2008 at 09:32:31AM +0200, Gildas wrote:
> 2008/3/29, Aurelien Jarno <aurelien@aurel32.net>:
> > On Wed, Mar 26, 2008 at 02:12:25PM +0100, Gildas wrote:
> > > This patch allows changing the boot device from within the monitor.
> >
> >
> > I have just updated BOCHS BIOS from upstream CVS. The new version allows
> > to select the boot device via a menu after the POST. I am therefore not
> > sure anymore that your patch is still useful. What do you think?
>
> Hi Aurelien,
>
> I see the monitor as some kind of iLO/management interface and I think
> the two approaches are somewhat orthogonal and have different use
> cases, so I believe they should be both present:
> - the boot menu after POST is user interactive
> - the command in the monitor may be interactive but can also be
> triggered by scripts (think automated installation such as FAI which
> boot on pxe then on the local drive) or a management layer (think
> libvirt).
>
> Also I wanted the entry in the monitor menu to be hardware independant
> so even if there is only x86 support for now it might be possible to
> use a single approach to set the boot device for all archs (at least
> the ones making use of the bootdevice parameter, such as sparc and
> ppc).
>
I see, make sense now. Please find my comments below.
> diff -Nur a/hw/pc.c b/hw/pc.c
> --- a/hw/pc.c 2008-03-18 07:53:05.000000000 +0100
> +++ b/hw/pc.c 2008-03-26 13:18:16.000000000 +0100
> @@ -180,6 +180,33 @@
> return 0;
> }
>
> +/* copy/pasted from cmos_init, should be made a general function
> + and used there as well */
> +int pc_set_bootdevice(const char *boot_device)
> +{
> +#define PC_MAX_BOOT_DEVICES 3
> + RTCState *s = rtc_state;
> + int nbds, bds[3] = { 0, };
> + int i;
> +
> + nbds = strlen(boot_device);
> + if (nbds > PC_MAX_BOOT_DEVICES) {
> + fprintf(stderr, "Too many boot devices for PC\n");
> + return(1);
The message is printed to stderr, while the command has been issued from
the monitor. Not very consistent.
> + }
> + for (i = 0; i < nbds; i++) {
> + bds[i] = boot_device2nibble(boot_device[i]);
> + if (bds[i] == 0) {
> + fprintf(stderr, "Invalid boot device for PC: '%c'\n",
> + boot_device[i]);
Same here.
> + return(1);
> + }
> + }
> + rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
> + rtc_set_memory(s, 0x38, (bds[2] << 4));
> + return(0);
> +}
> +
> /* hd_table must contain 4 block drivers */
> static void cmos_init(int ram_size, const char *boot_device,
> BlockDriverState **hd_table)
> {
> @@ -717,6 +744,8 @@
> BlockDriverState *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> BlockDriverState *fd[MAX_FD];
>
> + set_bootdevice_fct = pc_set_bootdevice;
> +
> linux_boot = (kernel_filename != NULL);
>
> /* init CPUs */
> diff -Nur a/monitor.c b/monitor.c
> --- a/monitor.c 2008-02-10 17:33:13.000000000 +0100
> +++ b/monitor.c 2008-03-26 13:17:56.000000000 +0100
> @@ -993,6 +993,21 @@
> suffix, addr, size * 2, val);
> }
>
> +static void do_bootdevice_set(const char *bootdevice)
> +{
> + int res;
> +
> + if (set_bootdevice_fct != NULL) {
> + res = set_bootdevice_fct(bootdevice);
> + if (res == 0)
> + term_printf("boot device list now set to %s\n", bootdevice);
> + else
> + term_printf("setting boot device list failed with error %i\n", res);
> + } else {
> + term_printf("no function defined to set boot device list for this architecture\n");
> + }
> +}
> +
> static void do_system_reset(void)
> {
> qemu_system_reset_request();
> @@ -1328,6 +1343,8 @@
> "capture index", "stop capture" },
> { "memsave", "lis", do_memory_save,
> "addr size file", "save to disk virtual memory dump starting at 'addr' of size 'size'", },
> + { "boot_set", "s", do_bootdevice_set,
> + "bootdevice", "define new values for the boot device list" },
> { NULL, NULL, },
> };
>
> diff -Nur a/sysemu.h b/sysemu.h
> --- a/sysemu.h 2008-03-18 07:53:05.000000000 +0100
> +++ b/sysemu.h 2008-03-26 13:12:20.000000000 +0100
> @@ -9,6 +9,11 @@
> extern int vm_running;
> extern const char *qemu_name;
>
> +/* handler to set the boot_device for a specific type of QEMUMachine */
> +/* return 0 if success */
> +typedef int QEMUMachineSetBootFunc(const char *boot_device);
> +QEMUMachineSetBootFunc *set_bootdevice_fct;
> +
Please don't declare variables in a .h. Multiple copies of the varialbe
would exists if the header is included multiple times, causing errors at
link.
Also, the other part of the code rely on it being defined to NULL by default.
> typedef struct vm_change_state_entry VMChangeStateEntry;
> typedef void VMChangeStateHandler(void *opaque, int running);
> typedef void VMStopHandler(void *opaque, int reason);
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-04-08 20:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-26 13:12 [Qemu-devel] [PATCH] allow bootdevice change from the monitor Gildas
2008-03-29 0:29 ` Aurelien Jarno
2008-03-30 7:32 ` Gildas
2008-04-08 20:54 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).