Re: [virtio-comment] Re: [PATCH v3 6/8] admin: Add theory of operation for write recording commands

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Nov 17, 2023 at 05:54:32PM +0800, Zhu, Lingshan wrote:
> 
> 
> On 11/17/2023 5:51 PM, Michael S. Tsirkin wrote:
> > On Fri, Nov 17, 2023 at 09:41:40AM +0000, Parav Pandit wrote:
> > > 
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Friday, November 17, 2023 3:08 PM
> > > > 
> > > > On Fri, Nov 17, 2023 at 09:14:21AM +0000, Parav Pandit wrote:
> > > > > 
> > > > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > > > Sent: Friday, November 17, 2023 2:16 PM In any case you can safely
> > > > > > assume that many users will have migration that takes seconds and
> > > > > > minutes.
> > > > > Strange, but ok. I don't see any problem with current method.
> > > > > 8MB is used for very large VM of 1TB takes minutes. Should be fine.
> > > > The problem is simple: vendors selling devices have no idea how large the VM
> > > > will be. So you have to over-provision for the max VM size.
> > > > If there was a way to instead allocate that in host memory, that would improve
> > > > on this.
> > > Not sure what to over provision for max VM size.
> > > Vendor does not know how many vcpus will be needed. It is no different problem.
> > > 
> > > When the VM migration is started, the individual tracking range is supplied by the hypervisor to device.
> > > Device allocates necessary memory on this instruction.
> > > 
> > > When the VM with certain size is provisioned, the member device can be provisioned for the VM size.
> > > And if it cannot be provisioned, possibly this may not the right member device to use at that point in time.
> > For someone who keeps arguing against adding single bit registers
> > "because it does not scale" you seem very nonchalant about adding
> > 8Mbytes.
> > 
> > I thought we have a nicely contained and orthogonal feature, so if it's
> > optional it's not a problem.
> > 
> > But with such costs and corner cases what exactly is the motivation for
> > the feature here?  Do you have a PoC showing how this works better than
> > e.g. shadow VQ?
> > 
> > Maybe IOMMU based and shadow VQ based tracking are the way to go
> > initially, and if there's a problem then we should add this later, on
> > top.
> I agree.

However, the patchset is ordered sensibly, first the device state
recording and then write tracking. So we can merge patches 1-5 and defer
6-8 if we want to.

Parav I suggest maybe split write tracking to a separate patchset just
because it seems so contentious.

I notice there have not been comments on 1-5 yet, I am not sure why
I started with patch 6 - I guess I was curious what it does.
I'll focus review on 1-5 next week.




> > 
> > I really want us to finally make progress merging features and anything
> > that reduces scope initially is good for that.
> >