Re: [RFC PATCH v1 07/25] hw/xen: Implement core serialize/deserialize methods for xenstore_impl

2023-03-07 Thread Paul Durrant

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

2023-03-07 Thread Paul Durrant

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

2023-03-07 Thread David Woodhouse
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

2023-03-07 Thread Paul Durrant

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

2023-03-07 Thread Paul Durrant

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

2023-03-07 Thread David Woodhouse
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

2023-03-07 Thread Paul Durrant

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