Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On 07/03/2023 16:59, Paul Durrant wrote:
On 07/03/2023 16:52, David Woodhouse wrote:
On Tue, 2023-03-07 at 16:39 +, Paul Durrant wrote:
On 07/03/2023 16:33, David Woodhouse wrote:
On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated?
Will
defer to Paul for that.
Signed-off-by: David Woodhouse
Paul, your Reviewed-by: on this one is conspicuous in its absence. I
mentioned migration in the cover letter — this much is working fine,
but it's the PV back ends that don't yet work.
I'd quite like to merge the basic serialization/deserialization of
XenStore itself, with the unit tests.
The patch is basically ok, I just think the serialization should be
limited to the guest nodes... filtering out those not owned by xen_domid
would probably work for that.
Yeah, so let's just do this (as part of this patch #7) for now:
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -235,6 +235,7 @@ static int xen_xenstore_post_load(void *opaque, int
ver)
static const VMStateDescription xen_xenstore_vmstate = {
.name = "xen_xenstore",
+ .unmigratable = 1, /* The PV back ends don't migrate yet */
.version_id = 1,
.minimum_version_id = 1,
.needed = xen_xenstore_is_needed,
It means we can't migrate guests even if they're only using fully
emulated devices... but I think that's a reasonable limitation until we
implement it fully.
Ok. With that added...
Revieweed-by: Paul Durrant
Typoed, sorry...
Reviewed-by: Paul Durrant
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On 07/03/2023 16:52, David Woodhouse wrote:
On Tue, 2023-03-07 at 16:39 +, Paul Durrant wrote:
On 07/03/2023 16:33, David Woodhouse wrote:
On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated? Will
defer to Paul for that.
Signed-off-by: David Woodhouse
Paul, your Reviewed-by: on this one is conspicuous in its absence. I
mentioned migration in the cover letter — this much is working fine,
but it's the PV back ends that don't yet work.
I'd quite like to merge the basic serialization/deserialization of
XenStore itself, with the unit tests.
The patch is basically ok, I just think the serialization should be
limited to the guest nodes... filtering out those not owned by xen_domid
would probably work for that.
Yeah, so let's just do this (as part of this patch #7) for now:
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -235,6 +235,7 @@ static int xen_xenstore_post_load(void *opaque, int
ver)
static const VMStateDescription xen_xenstore_vmstate = {
.name = "xen_xenstore",
+.unmigratable = 1, /* The PV back ends don't migrate yet */
.version_id = 1,
.minimum_version_id = 1,
.needed = xen_xenstore_is_needed,
It means we can't migrate guests even if they're only using fully
emulated devices... but I think that's a reasonable limitation until we
implement it fully.
Ok. With that added...
Revieweed-by: Paul Durrant
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On Tue, 2023-03-07 at 16:39 +, Paul Durrant wrote:
> On 07/03/2023 16:33, David Woodhouse wrote:
> > On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
> > > From: David Woodhouse
> > >
> > > In fact I think we want to only serialize the contents of the domain's
> > > path in /local/domain/${domid} and leave the rest to be recreated? Will
> > > defer to Paul for that.
> > >
> > > Signed-off-by: David Woodhouse
> >
> > Paul, your Reviewed-by: on this one is conspicuous in its absence. I
> > mentioned migration in the cover letter — this much is working fine,
> > but it's the PV back ends that don't yet work.
> >
> > I'd quite like to merge the basic serialization/deserialization of
> > XenStore itself, with the unit tests.
>
> The patch is basically ok, I just think the serialization should be
> limited to the guest nodes... filtering out those not owned by xen_domid
> would probably work for that.
Yeah, so let's just do this (as part of this patch #7) for now:
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -235,6 +235,7 @@ static int xen_xenstore_post_load(void *opaque, int
ver)
static const VMStateDescription xen_xenstore_vmstate = {
.name = "xen_xenstore",
+.unmigratable = 1, /* The PV back ends don't migrate yet */
.version_id = 1,
.minimum_version_id = 1,
.needed = xen_xenstore_is_needed,
It means we can't migrate guests even if they're only using fully
emulated devices... but I think that's a reasonable limitation until we
implement it fully.
smime.p7s
Description: S/MIME cryptographic signature
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On 07/03/2023 16:39, Paul Durrant wrote:
On 07/03/2023 16:33, David Woodhouse wrote:
On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated? Will
defer to Paul for that.
Signed-off-by: David Woodhouse
Paul, your Reviewed-by: on this one is conspicuous in its absence. I
mentioned migration in the cover letter — this much is working fine,
but it's the PV back ends that don't yet work.
I'd quite like to merge the basic serialization/deserialization of
XenStore itself, with the unit tests.
The patch is basically ok, I just think the serialization should be
limited to the guest nodes... filtering out those not owned by xen_domid
would probably work for that.
Perhaps we can also set TYPE_XEN_DEVICE or TYPE_XEN_BLOCK_DEVICE to be
unmigratable? Ideally I think we want TYPE_XEN_DEVICE to be
unmigratable by default *unless* the specific device class (including
net and other as we port them from XenLegacyDevice) says otherwise.
Yes, that sounds like a good idea.
Is there a way to do that?
Not something I've looked into. I'll go look now.
Maybe calling migrate_add_blocker() in the realize method is the right
way to achieve this?
Paul
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On 07/03/2023 16:33, David Woodhouse wrote:
On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated? Will
defer to Paul for that.
Signed-off-by: David Woodhouse
Paul, your Reviewed-by: on this one is conspicuous in its absence. I
mentioned migration in the cover letter — this much is working fine,
but it's the PV back ends that don't yet work.
I'd quite like to merge the basic serialization/deserialization of
XenStore itself, with the unit tests.
The patch is basically ok, I just think the serialization should be
limited to the guest nodes... filtering out those not owned by xen_domid
would probably work for that.
Perhaps we can also set TYPE_XEN_DEVICE or TYPE_XEN_BLOCK_DEVICE to be
unmigratable? Ideally I think we want TYPE_XEN_DEVICE to be
unmigratable by default *unless* the specific device class (including
net and other as we port them from XenLegacyDevice) says otherwise.
Yes, that sounds like a good idea.
Is there a way to do that?
Not something I've looked into. I'll go look now.
Paul
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On Thu, 2023-03-02 at 15:34 +, David Woodhouse wrote:
> From: David Woodhouse
>
> In fact I think we want to only serialize the contents of the domain's
> path in /local/domain/${domid} and leave the rest to be recreated? Will
> defer to Paul for that.
>
> Signed-off-by: David Woodhouse
Paul, your Reviewed-by: on this one is conspicuous in its absence. I
mentioned migration in the cover letter — this much is working fine,
but it's the PV back ends that don't yet work.
I'd quite like to merge the basic serialization/deserialization of
XenStore itself, with the unit tests.
Perhaps we can also set TYPE_XEN_DEVICE or TYPE_XEN_BLOCK_DEVICE to be
unmigratable? Ideally I think we want TYPE_XEN_DEVICE to be
unmigratable by default *unless* the specific device class (including
net and other as we port them from XenLegacyDevice) says otherwise.
Is there a way to do that?
smime.p7s
Description: S/MIME cryptographic signature
Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
On 02/03/2023 15:34, David Woodhouse wrote:
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated? Will
defer to Paul for that.
Yes. Assuming backends have properly implemented save/restore code then
they should be able to re-create their xenstore content. Also, it's
generally convenient for them to start back in 'InitWait' state the
drive their state machine back to 'Connected' so that grant refs can be
mapped, event channels bound etc. along the way.
Signed-off-by: David Woodhouse
---
hw/i386/kvm/xen_xenstore.c | 25 +-
hw/i386/kvm/xenstore_impl.c | 574 +++-
hw/i386/kvm/xenstore_impl.h | 5 +
tests/unit/test-xs-node.c | 236 ++-
4 files changed, 824 insertions(+), 16 deletions(-)
The code LGTM though; just need to limit the saved tree to
'/local/domain/'
Paul
[RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl
From: David Woodhouse
In fact I think we want to only serialize the contents of the domain's
path in /local/domain/${domid} and leave the rest to be recreated? Will
defer to Paul for that.
Signed-off-by: David Woodhouse
---
hw/i386/kvm/xen_xenstore.c | 25 +-
hw/i386/kvm/xenstore_impl.c | 574 +++-
hw/i386/kvm/xenstore_impl.h | 5 +
tests/unit/test-xs-node.c | 236 ++-
4 files changed, 824 insertions(+), 16 deletions(-)
diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 3b409e3817..1b1358ad4c 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -66,6 +66,9 @@ struct XenXenstoreState {
evtchn_port_t guest_port;
evtchn_port_t be_port;
struct xenevtchn_handle *eh;
+
+uint8_t *impl_state;
+uint32_t impl_state_size;
};
struct XenXenstoreState *xen_xenstore_singleton;
@@ -109,16 +112,26 @@ static bool xen_xenstore_is_needed(void *opaque)
static int xen_xenstore_pre_save(void *opaque)
{
XenXenstoreState *s = opaque;
+GByteArray *save;
if (s->eh) {
s->guest_port = xen_be_evtchn_get_guest_port(s->eh);
}
+
+g_free(s->impl_state);
+save = xs_impl_serialize(s->impl);
+s->impl_state = save->data;
+s->impl_state_size = save->len;
+g_byte_array_free(save, false);
+
return 0;
}
static int xen_xenstore_post_load(void *opaque, int ver)
{
XenXenstoreState *s = opaque;
+GByteArray *save;
+int ret;
/*
* As qemu/dom0, rebind to the guest's port. The Windows drivers may
@@ -135,7 +148,13 @@ static int xen_xenstore_post_load(void *opaque, int ver)
}
s->be_port = be_port;
}
-return 0;
+
+save = g_byte_array_new_take(s->impl_state, s->impl_state_size);
+s->impl_state = NULL;
+s->impl_state_size = 0;
+
+ret = xs_impl_deserialize(s->impl, save, xen_domid, fire_watch_cb, s);
+return ret;
}
static const VMStateDescription xen_xenstore_vmstate = {
@@ -155,6 +174,10 @@ static const VMStateDescription xen_xenstore_vmstate = {
VMSTATE_BOOL(rsp_pending, XenXenstoreState),
VMSTATE_UINT32(guest_port, XenXenstoreState),
VMSTATE_BOOL(fatal_error, XenXenstoreState),
+VMSTATE_UINT32(impl_state_size, XenXenstoreState),
+VMSTATE_VARRAY_UINT32_ALLOC(impl_state, XenXenstoreState,
+impl_state_size, 0,
+vmstate_info_uint8, uint8_t),
VMSTATE_END_OF_LIST()
}
};
diff --git a/hw/i386/kvm/xenstore_impl.c b/hw/i386/kvm/xenstore_impl.c
index 7988bde88f..82e7ae06f5 100644
--- a/hw/i386/kvm/xenstore_impl.c
+++ b/hw/i386/kvm/xenstore_impl.c
@@ -37,6 +37,7 @@ typedef struct XsNode {
uint64_t gencnt;
bool deleted_in_tx;
bool modified_in_tx;
+unsigned int serialized_tx;
#ifdef XS_NODE_UNIT_TEST
gchar *name; /* debug only */
#endif
@@ -68,6 +69,7 @@ struct XenstoreImplState {
unsigned int nr_domu_transactions;
unsigned int root_tx;
unsigned int last_tx;
+bool serialized;
};
@@ -1156,8 +1158,10 @@ int xs_impl_set_perms(XenstoreImplState *s, unsigned int
dom_id,
return xs_node_walk(n, &op);
}
-int xs_impl_watch(XenstoreImplState *s, unsigned int dom_id, const char *path,
- const char *token, xs_impl_watch_fn fn, void *opaque)
+static int do_xs_impl_watch(XenstoreImplState *s, unsigned int dom_id,
+const char *path, const char *token,
+xs_impl_watch_fn fn, void *opaque)
+
{
char abspath[XENSTORE_ABS_PATH_MAX + 1];
XsWatch *w, *l;
@@ -1200,12 +1204,22 @@ int xs_impl_watch(XenstoreImplState *s, unsigned int
dom_id, const char *path,
s->nr_domu_watches++;
}
-/* A new watch should fire immediately */
-fn(opaque, path, token);
-
return 0;
}
+int xs_impl_watch(XenstoreImplState *s, unsigned int dom_id, const char *path,
+ const char *token, xs_impl_watch_fn fn, void *opaque)
+{
+int ret = do_xs_impl_watch(s, dom_id, path, token, fn, opaque);
+
+if (!ret) {
+/* A new watch should fire immediately */
+fn(opaque, path, token);
+}
+
+return ret;
+}
+
static XsWatch *free_watch(XenstoreImplState *s, XsWatch *w)
{
XsWatch *next = w->next;
@@ -1361,3 +1375,553 @@ XenstoreImplState *xs_impl_create(unsigned int dom_id)
s->root_tx = s->last_tx = 1;
return s;
}
+
+
+static void clear_serialized_tx(gpointer key, gpointer value, gpointer opaque)
+{
+XsNode *n = value;
+
+n->serialized_tx = XBT_NULL;
+if (n->children) {
+g_hash_table_foreach(n->children, clear_serialized_tx, NULL);
+}
+}
+
+static void clear_tx_serialized_tx(gpointer key, gpointer value,
+ gpointer opaque)
+{
+XsTransaction *t = value;
+
+clear_serialized_tx(NULL, t->root, NULL);
+}
+
+static void write_be32(GByteArray *save, u
