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

[Parav Pandit <parav@nvidia.com>]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Michael S. Tsirkin
> Sent: Friday, November 17, 2023 3:32 PM
> 
> 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.

Ok. sounds good.