| Message ID | 1309346169-14554-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On 06/29/2011 01:16 PM, stefano.stabellini@eu.citrix.com wrote: > From: Anthony PERARD<anthony.perard@citrix.com> > > Remove the call to xenstore_record_dm_state from xen_main_loop_prepare > that is HVM specific. > Add a new vm_change_state_handler shared between xen_pv and xen_hvm > machines to record the VM state to xenstore. > > Signed-off-by: Anthony PERARD<anthony.perard@citrix.com> > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > --- > xen-all.c | 25 ++++++++++++++++++------- > 1 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/xen-all.c b/xen-all.c > index 3fd04ef..e8da35f 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) > } > } > > -static void xenstore_record_dm_state(XenIOState *s, const char *state) > +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > { > char path[50]; > > + if (xs == NULL) { > + fprintf(stderr, "xenstore connection not initialized\n"); > + exit(1); > + } > + > snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); > - if (!xs_write(s->xenstore, XBT_NULL, path, state, strlen(state))) { > + if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { > fprintf(stderr, "error recording dm state\n"); > exit(1); > } > @@ -823,15 +828,20 @@ static void xen_main_loop_prepare(XenIOState *state) > if (evtchn_fd != -1) { > qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state); > } > - > - /* record state running */ > - xenstore_record_dm_state(state, "running"); > } > > > /* Initialise Xen */ > > -static void xen_vm_change_state_handler(void *opaque, int running, int reason) > +static void xen_change_state_handler(void *opaque, int running, int reason) > +{ > + if (running) { > + /* record state running */ > + xenstore_record_dm_state(xenstore, "running"); > + } So where is this ever executed in the PV case? Alex
On 06/30/2011 04:37 PM, Stefano Stabellini wrote: > On Thu, 30 Jun 2011, Alexander Graf wrote: >> On 06/29/2011 01:16 PM, stefano.stabellini@eu.citrix.com wrote: >>> From: Anthony PERARD<anthony.perard@citrix.com> >>> >>> Remove the call to xenstore_record_dm_state from xen_main_loop_prepare >>> that is HVM specific. >>> Add a new vm_change_state_handler shared between xen_pv and xen_hvm >>> machines to record the VM state to xenstore. >>> >>> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com> >>> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> >>> --- >>> xen-all.c | 25 ++++++++++++++++++------- >>> 1 files changed, 18 insertions(+), 7 deletions(-) >>> >>> diff --git a/xen-all.c b/xen-all.c >>> index 3fd04ef..e8da35f 100644 >>> --- a/xen-all.c >>> +++ b/xen-all.c >>> @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) >>> } >>> } >>> >>> -static void xenstore_record_dm_state(XenIOState *s, const char *state) >>> +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) >>> { >>> char path[50]; >>> >>> + if (xs == NULL) { >>> + fprintf(stderr, "xenstore connection not initialized\n"); >>> + exit(1); >>> + } >>> + >>> snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); >>> - if (!xs_write(s->xenstore, XBT_NULL, path, state, strlen(state))) { >>> + if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { >>> fprintf(stderr, "error recording dm state\n"); >>> exit(1); >>> } >>> @@ -823,15 +828,20 @@ static void xen_main_loop_prepare(XenIOState *state) >>> if (evtchn_fd != -1) { >>> qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state); >>> } >>> - >>> - /* record state running */ >>> - xenstore_record_dm_state(state, "running"); >>> } >>> >>> >>> /* Initialise Xen */ >>> >>> -static void xen_vm_change_state_handler(void *opaque, int running, int reason) >>> +static void xen_change_state_handler(void *opaque, int running, int reason) >>> +{ >>> + if (running) { >>> + /* record state running */ >>> + xenstore_record_dm_state(xenstore, "running"); >>> + } >> So where is this ever executed in the PV case? > > qemu_add_vm_change_state_handler(xen_change_state_handler, NULL) is now > called by xen_init, that is common between xen_pv and xen_fv machines Ah, there it is. Sorry, I missed it the first time I read the patch :). Alex
On Thu, 30 Jun 2011, Alexander Graf wrote: > On 06/29/2011 01:16 PM, stefano.stabellini@eu.citrix.com wrote: > > From: Anthony PERARD<anthony.perard@citrix.com> > > > > Remove the call to xenstore_record_dm_state from xen_main_loop_prepare > > that is HVM specific. > > Add a new vm_change_state_handler shared between xen_pv and xen_hvm > > machines to record the VM state to xenstore. > > > > Signed-off-by: Anthony PERARD<anthony.perard@citrix.com> > > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > > --- > > xen-all.c | 25 ++++++++++++++++++------- > > 1 files changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/xen-all.c b/xen-all.c > > index 3fd04ef..e8da35f 100644 > > --- a/xen-all.c > > +++ b/xen-all.c > > @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) > > } > > } > > > > -static void xenstore_record_dm_state(XenIOState *s, const char *state) > > +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > > { > > char path[50]; > > > > + if (xs == NULL) { > > + fprintf(stderr, "xenstore connection not initialized\n"); > > + exit(1); > > + } > > + > > snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); > > - if (!xs_write(s->xenstore, XBT_NULL, path, state, strlen(state))) { > > + if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { > > fprintf(stderr, "error recording dm state\n"); > > exit(1); > > } > > @@ -823,15 +828,20 @@ static void xen_main_loop_prepare(XenIOState *state) > > if (evtchn_fd != -1) { > > qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state); > > } > > - > > - /* record state running */ > > - xenstore_record_dm_state(state, "running"); > > } > > > > > > /* Initialise Xen */ > > > > -static void xen_vm_change_state_handler(void *opaque, int running, int reason) > > +static void xen_change_state_handler(void *opaque, int running, int reason) > > +{ > > + if (running) { > > + /* record state running */ > > + xenstore_record_dm_state(xenstore, "running"); > > + } > > So where is this ever executed in the PV case? qemu_add_vm_change_state_handler(xen_change_state_handler, NULL) is now called by xen_init, that is common between xen_pv and xen_fv machines
On 06/29/2011 01:16 PM, stefano.stabellini@eu.citrix.com wrote: > From: Anthony PERARD<anthony.perard@citrix.com> > > Remove the call to xenstore_record_dm_state from xen_main_loop_prepare > that is HVM specific. > Add a new vm_change_state_handler shared between xen_pv and xen_hvm > machines to record the VM state to xenstore. > > Signed-off-by: Anthony PERARD<anthony.perard@citrix.com> > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > --- > xen-all.c | 25 ++++++++++++++++++------- > 1 files changed, 18 insertions(+), 7 deletions(-) > > diff --git a/xen-all.c b/xen-all.c > index 3fd04ef..e8da35f 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) > } > } > > -static void xenstore_record_dm_state(XenIOState *s, const char *state) > +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > { > char path[50]; > > + if (xs == NULL) { > + fprintf(stderr, "xenstore connection not initialized\n"); > + exit(1); > + } /studio/tmp/agraf/xen-all.c: In function ‘xenstore_record_dm_state’: /studio/tmp/agraf/xen-all.c:744: error: ‘xs’ undeclared (first use in this function) /studio/tmp/agraf/xen-all.c:744: error: (Each undeclared identifier is reported only once /studio/tmp/agraf/xen-all.c:744: error: for each function it appears in.) Alex
I have just realized that I wrote the reply to this email long ago but I have never sent it, it was sitting in my Draft mbox. On Thu, 30 Jun 2011, Alexander Graf wrote: > On 06/29/2011 01:16 PM, stefano.stabellini@eu.citrix.com wrote: > > From: Anthony PERARD<anthony.perard@citrix.com> > > > > Remove the call to xenstore_record_dm_state from xen_main_loop_prepare > > that is HVM specific. > > Add a new vm_change_state_handler shared between xen_pv and xen_hvm > > machines to record the VM state to xenstore. > > > > Signed-off-by: Anthony PERARD<anthony.perard@citrix.com> > > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > > --- > > xen-all.c | 25 ++++++++++++++++++------- > > 1 files changed, 18 insertions(+), 7 deletions(-) > > > > diff --git a/xen-all.c b/xen-all.c > > index 3fd04ef..e8da35f 100644 > > --- a/xen-all.c > > +++ b/xen-all.c > > @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) > > } > > } > > > > -static void xenstore_record_dm_state(XenIOState *s, const char *state) > > +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > > { > > char path[50]; > > > > + if (xs == NULL) { > > + fprintf(stderr, "xenstore connection not initialized\n"); > > + exit(1); > > + } > > /studio/tmp/agraf/xen-all.c: In function ‘xenstore_record_dm_state’: > /studio/tmp/agraf/xen-all.c:744: error: ‘xs’ undeclared (first use in > this function) > /studio/tmp/agraf/xen-all.c:744: error: (Each undeclared identifier is > reported only once > /studio/tmp/agraf/xen-all.c:744: error: for each function it appears in.) I think you had some conflicts applying the patch, I'll post it again rebased on your xen-next branch.
diff --git a/xen-all.c b/xen-all.c index 3fd04ef..e8da35f 100644 --- a/xen-all.c +++ b/xen-all.c @@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, CharDriverState *chr) } } -static void xenstore_record_dm_state(XenIOState *s, const char *state) +static void xenstore_record_dm_state(struct xs_handle *xs, const char *state) { char path[50]; + if (xs == NULL) { + fprintf(stderr, "xenstore connection not initialized\n"); + exit(1); + } + snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", xen_domid); - if (!xs_write(s->xenstore, XBT_NULL, path, state, strlen(state))) { + if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) { fprintf(stderr, "error recording dm state\n"); exit(1); } @@ -823,15 +828,20 @@ static void xen_main_loop_prepare(XenIOState *state) if (evtchn_fd != -1) { qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state); } - - /* record state running */ - xenstore_record_dm_state(state, "running"); } /* Initialise Xen */ -static void xen_vm_change_state_handler(void *opaque, int running, int reason) +static void xen_change_state_handler(void *opaque, int running, int reason) +{ + if (running) { + /* record state running */ + xenstore_record_dm_state(xenstore, "running"); + } +} + +static void xen_hvm_change_state_handler(void *opaque, int running, int reason) { XenIOState *state = opaque; if (running) { @@ -854,6 +864,7 @@ int xen_init(void) xen_be_printf(NULL, 0, "can't open xen interface\n"); return -1; } + qemu_add_vm_change_state_handler(xen_change_state_handler, NULL); return 0; } @@ -915,7 +926,7 @@ int xen_hvm_init(void) qemu_map_cache_init(); xen_ram_init(ram_size); - qemu_add_vm_change_state_handler(xen_vm_change_state_handler, state); + qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state); state->client = xen_cpu_phys_memory_client; QLIST_INIT(&state->physmap);