On Fri, Oct 10, 2025 at 2:27 AM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> On 10.10.25 02:43, Raphael Norwitz wrote:
> > On Thu, Oct 9, 2025 at 5:14 PM Vladimir Sementsov-Ogievskiy
> > <[email protected]> wrote:
> >>
> >> On 09.10.25 22:09, Raphael Norwitz wrote:
> >>> A small question here but will review more thoroughly pending feedback
> >>> on my overall comments.
> >>>
> >>
> >> I really hope you didn't spent much time on these 28-31 patches :/
> >>
> >
> > I spent much more time on the cleanups :)
> >
> >>> On Wed, Aug 13, 2025 at 12:53 PM Vladimir Sementsov-Ogievskiy
> >>> <[email protected]> wrote:
> >>>>
> >>
> >> [..]
> >>
> >>>> --- a/migration/options.c
> >>>> +++ b/migration/options.c
> >>>> @@ -269,6 +269,13 @@ bool migrate_local_char_socket(void)
> >>>>        return s->capabilities[MIGRATION_CAPABILITY_LOCAL_CHAR_SOCKET];
> >>>>    }
> >>>>
> >>>> +bool migrate_local_vhost_user_blk(void)
> >>>> +{
> >>>> +    MigrationState *s = migrate_get_current();
> >>>> +
> >>>
> >>> Where was MIGRATION_CAPABILITY_LOCAL_VHOST_USER_BLK added/defined?
> >>
> >> It is generated by QAPI code generator.
> >>
> >> Exactly, it's defined by 'local-vhost-user-blk' member inside 
> >> 'MigrationCapability':
> >>
> >> { 'enum': 'MigrationCapability',
> >>     'data': ['xbzrle', 'rdma-pin-all', 'auto-converge',
> >>
> >>              ...
> >>
> >>              { 'name': 'local-vhost-user-blk', 'features': [ 'unstable' ] 
> >> } ] }
> >>
> >>
> >> and after build, the generated code is in 
> >> build/qapi/qapi-types-migration.h, as a enum:
> >>
> >> typedef enum MigrationCapability {
> >>       MIGRATION_CAPABILITY_XBZRLE,
> >>
> >>       ,,,
> >>
> >>       MIGRATION_CAPABILITY_LOCAL_VHOST_USER_BLK,
> >>       MIGRATION_CAPABILITY__MAX,
> >> } MigrationCapability;
> >>
> >>
> >> In v2, I'll follow the interface of virtio-net series, look at
> >>
> >> https://patchew.org/QEMU/[email protected]/[email protected]/
> >>
> >> so, it would be migration parameter instead of capability, like
> >>
> >>       QMP migrate-set-parameters {... backend-transfer = 
> >> ["vhost-user-blk"] }
> >>
> >> and to enable both vhost-user-blk and virtio-net-tap together:
> >>
> >>       QMP migrate-set-parameters {... backend-transfer = 
> >> ["vhost-user-blk", "virtio-net-tap"] }
> >>
> >
> > Why do we need two separate migration parameters for vhost-user-blk
> > and virtio-net-tap? Why not have a single parameter for virtio local
> > migrations and, if it is set, all backends types which support local
> > migration can advertise and take advantage of it?
>
> As I describe in the commit message 
> https://patchew.org/QEMU/[email protected]/[email protected]/
>  :
>
>
> Why not simple boolean? To simplify migration to further versions,
> when more devices will support backend-transfer migration.
>
> Alternatively, we may add per-device option to disable backend-transfer
> migration, but still:
>
> 1. It's more comfortable to set same capabilities/parameters on both
> source and target QEMU, than care about each device.
>
> 2. To not break the design, that machine-type + device options +
> migration capabilities and parameters are fully define the resulting
> migration stream. We'll break this if add in future more
> backend-transfer support in devices under same backend-transfer=true
> parameter.

ACK on needing a separate migration parameter. Thanks for the references.

I would suggest having the incoming_backend field in the struct
vhost_user (or maybe even in struct vhost_dev if the tap device
migration is similar enough) rather than in struct VHostUserBlk, so
that device-specific code can be kept as similar as possible.

>
>
> >
> >>>
> >>>
> >>>> +    return s->capabilities[MIGRATION_CAPABILITY_LOCAL_VHOST_USER_BLK];
> >>>> +}
> >>>> +
> >>
> >> [..]
> >>
> >>
> >> --
> >> Best regards,
> >> Vladimir
>
>
> --
> Best regards,
> Vladimir

Reply via email to