Re: Virtio-disk updates for latest Xen ?

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 15.12.22 06:22, Viresh Kumar wrote:

Hello Viresh


> On 14-12-22, 17:01, Oleksandr Tyshchenko wrote:
> > Today I had a chance to check virtio-disk on my H/W using new Xen branch
> > which does include Juergen's series with commit 3a96013a3e17
> > ("tools/xenstore: reduce number of watch events").
> >
> > Very interesting, but I didn't manage to reproduce an issue the similar to
> > what you had already faced with the rust counterparts before (caused by the
> > lack of Xenstore watches?). Note that I didn't debug what exactly events I
> > had got during guest creation/destruction, I just made sure that backend
> > worked as before. I checked that by running the backend in Dom0 and DomD and
> > performed a couple of guest power cycles (reboot, destroy/create).
> >
> > If you could provide the debug patch which you seem to use to print incoming
> > events which you described in previous email, I think I would be able to
> > re-check the situation at my side more deeper.
> This should be enough to see the new changes I believe.
>
> diff --git a/xs_dev.c b/xs_dev.c
> index a6c8403cfe84..8525c6512299 100755
> --- a/xs_dev.c
> +++ b/xs_dev.c
> @@ -421,6 +421,8 @@ static int xenstore_poll_be_watch(struct xs_dev *dev)
>       if (!vec)
>           return -1;
>   
> +    printf("%s: %s\n", vec[XS_WATCH_PATH], dev->path);
> +
>       if (!strcmp(vec[XS_WATCH_PATH], dev->path))
>           rc = xenstore_get_fe_domid(dev);

Thanks. This print does not provide much information in my case 
(virtio-disk). All what I see here in both cases (with old and new Xen) 
during guest creation is the following:
"backend/virtio_disk: backend/virtio_disk"

But, if I modify the xenstore_get_fe_domid() to always return 0 and as 
the result to get stuck here, I can see subsequent events for other 
paths here.
I agree that with new Xen (after commit 3a96013a3e17 "tools/xenstore: 
reduce number of watch events"), some events are missing now, but I
still don't see an issue with virtio-disk and can't see why it is going 
to be an issue. The code doesn't wait for the last "finalizing" event 
for the root directory "backend/virtio_disk", it goes ahead to find the 
FE domid right after receiving the first event.
new Xen:
oot@generic-armv8-xt-dom0:~# xl console DomD
backend/virtio_disk/6/51713: backend/virtio_disk
backend/virtio_disk/6/51713/frontend: backend/virtio_disk
backend/virtio_disk/6/51713/params: backend/virtio_disk
backend/virtio_disk/6/51713/frontend-id: backend/virtio_disk
backend/virtio_disk/6/51713/online: backend/virtio_disk
backend/virtio_disk/6/51713/removable: backend/virtio_disk
backend/virtio_disk/6/51713/bootable: backend/virtio_disk
backend/virtio_disk/6/51713/state: backend/virtio_disk
backend/virtio_disk/6/51713/dev: backend/virtio_disk
backend/virtio_disk/6/51713/type: backend/virtio_disk
backend/virtio_disk/6/51713/mode: backend/virtio_disk
backend/virtio_disk/6/51713/device-type: backend/virtio_disk
backend/virtio_disk/6/51713/discard-enable: backend/virtio_disk
backend/virtio_disk/6/51713/specification: backend/virtio_disk
backend/virtio_disk/6/51713/transport: backend/virtio_disk
backend/virtio_disk/6/51713/base: backend/virtio_disk
backend/virtio_disk/6/51713/irq: backend/virtio_disk

old Xen:
root@generic-armv8-xt-dom0:~# xl console DomD
backend/virtio_disk/4/51713: backend/virtio_disk
backend/virtio_disk/4: backend/virtio_disk
backend/virtio_disk: backend/virtio_disk
backend/virtio_disk/4/51713/frontend: backend/virtio_disk
backend/virtio_disk/4/51713/params: backend/virtio_disk
backend/virtio_disk/4/51713/frontend-id: backend/virtio_disk
backend/virtio_disk/4/51713/online: backend/virtio_disk
backend/virtio_disk/4/51713/removable: backend/virtio_disk
backend/virtio_disk/4/51713/bootable: backend/virtio_disk
backend/virtio_disk/4/51713/state: backend/virtio_disk
backend/virtio_disk/4/51713/dev: backend/virtio_disk
backend/virtio_disk/4/51713/type: backend/virtio_disk
backend/virtio_disk/4/51713/mode: backend/virtio_disk
backend/virtio_disk/4/51713/device-type: backend/virtio_disk
backend/virtio_disk/4/51713/discard-enable: backend/virtio_disk
backend/virtio_disk/4/51713/specification: backend/virtio_disk
backend/virtio_disk/4/51713/transport: backend/virtio_disk
backend/virtio_disk/4/51713/base: backend/virtio_disk
backend/virtio_disk/4/51713/irq: backend/virtio_disk

At the same time the code to automatically determine the FE domid is not 
ideal, for the instance, it is based on the assumption
that new FE domid should be always greater than old FE domid (which as I 
understand *might* be wrong) and contains a delay, so wants to be improved.