Re: [PATCH 3/3] usbredir: avoid queuing hello packet on snapshot restore

[Victor Toso]

Hi,

On Fri, Aug 12, 2022 at 10:33:54PM -0700, Joelle van Dyne wrote:
> On Fri, Aug 12, 2022 at 10:30 PM Victor Toso <victortoso@redhat.com> wrote:
> >
> > Hi,
> >
> > On Fri, Aug 12, 2022 at 06:10:31PM -0700, Joelle van Dyne wrote:
> > > When launching QEMU with "-loadvm", usbredir_create_parser() should avoid
> > > setting up the hello packet (just as with "-incoming". On the latest 
> > > version
> > > of libusbredir, usbredirparser_unserialize() will return error if the 
> > > parser
> > > is not "pristine."
> >
> > That was wrong in the usbredir side. The fix [0] was merged and
> > included in the latest 0.13.0 release
>
> This is good to know. Should the entire runstate_check in
> usbredir_create_parser be removed?

From my POV your patch looks correct and would avoid migration
issues such as [1] with usbredir 0.12.0 that was not patched

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2096008

So, I'd keep the check and the patch :)

> > [0] https://gitlab.freedesktop.org/spice/usbredir/-/merge_requests/61
> >
> > > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > > ---
> > >  hw/usb/redirect.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> > > index fd7df599bc..47fac3895a 100644
> > > --- a/hw/usb/redirect.c
> > > +++ b/hw/usb/redirect.c
> > > @@ -1280,7 +1280,8 @@ static void usbredir_create_parser(USBRedirDevice 
> > > *dev)
> > >      }
> > >  #endif
> > >
> > > -    if (runstate_check(RUN_STATE_INMIGRATE)) {
> > > +    if (runstate_check(RUN_STATE_INMIGRATE) ||
> > > +        runstate_check(RUN_STATE_RESTORE_VM)) {
> > >          flags |= usbredirparser_fl_no_hello;
> > >      }
> > >      usbredirparser_init(dev->parser, VERSION, caps, USB_REDIR_CAPS_SIZE,
> > > --
> > > 2.28.0
> > >
> > >

Cheers,
Victor