Message ID | 20211026090605.91646-7-yishaih@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Bjorn Helgaas |
Headers | show |
Series | Add mlx5 live migration driver | expand |
On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE > instead of STATE). > > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> This s-o-b chain looks weird; your s-o-b always needs to be last. > --- > include/uapi/linux/vfio.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index ef33ea002b0b..114ffcefe437 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -622,7 +622,7 @@ struct vfio_device_migration_info { > VFIO_DEVICE_STATE_RESUMING)) > > #define VFIO_DEVICE_STATE_SET_ERROR(state) \ > - ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_SATE_SAVING | \ > + ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_STATE_SAVING | \ > VFIO_DEVICE_STATE_RESUMING) > > __u32 reserved; Change looks fine, although we might consider merging it with the next patch? Anyway, Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Tue, 26 Oct 2021 17:32:19 +0200 Cornelia Huck <cohuck@redhat.com> wrote: > On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: > > > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE > > instead of STATE). > > > > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > This s-o-b chain looks weird; your s-o-b always needs to be last. > > > --- > > include/uapi/linux/vfio.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > index ef33ea002b0b..114ffcefe437 100644 > > --- a/include/uapi/linux/vfio.h > > +++ b/include/uapi/linux/vfio.h > > @@ -622,7 +622,7 @@ struct vfio_device_migration_info { > > VFIO_DEVICE_STATE_RESUMING)) > > > > #define VFIO_DEVICE_STATE_SET_ERROR(state) \ > > - ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_SATE_SAVING | \ > > + ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_STATE_SAVING | \ > > VFIO_DEVICE_STATE_RESUMING) > > > > __u32 reserved; > > Change looks fine, although we might consider merging it with the next > patch? Anyway, I had requested it separate a couple revisions ago since it's a fix. Thanks, Alex
On Tue, Oct 26 2021, Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 26 Oct 2021 17:32:19 +0200 > Cornelia Huck <cohuck@redhat.com> wrote: > >> On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: >> >> > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE >> > instead of STATE). >> > >> > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") >> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> >> >> This s-o-b chain looks weird; your s-o-b always needs to be last. >> >> > --- >> > include/uapi/linux/vfio.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >> > index ef33ea002b0b..114ffcefe437 100644 >> > --- a/include/uapi/linux/vfio.h >> > +++ b/include/uapi/linux/vfio.h >> > @@ -622,7 +622,7 @@ struct vfio_device_migration_info { >> > VFIO_DEVICE_STATE_RESUMING)) >> > >> > #define VFIO_DEVICE_STATE_SET_ERROR(state) \ >> > - ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_SATE_SAVING | \ >> > + ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_STATE_SAVING | \ >> > VFIO_DEVICE_STATE_RESUMING) >> > >> > __u32 reserved; >> >> Change looks fine, although we might consider merging it with the next >> patch? Anyway, > > I had requested it separate a couple revisions ago since it's a fix. > Thanks, > > Alex Fair enough.
On Tue, Oct 26, 2021 at 05:32:19PM +0200, Cornelia Huck wrote: > On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: > > > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE > > instead of STATE). > > > > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") > > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > > This s-o-b chain looks weird; your s-o-b always needs to be last. It is not such clear as it sounds. Yishai is author of this patch and at some point of time, this patch passed through my tree and it will pass again, when we will merge it. This is why my SOB is last and not Yishai's. Thanks
On Tue, Oct 26 2021, Leon Romanovsky <leonro@nvidia.com> wrote: > On Tue, Oct 26, 2021 at 05:32:19PM +0200, Cornelia Huck wrote: >> On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: >> >> > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE >> > instead of STATE). >> > >> > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") >> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> >> >> This s-o-b chain looks weird; your s-o-b always needs to be last. > > It is not such clear as it sounds. > > Yishai is author of this patch and at some point of time, this patch passed > through my tree and it will pass again, when we will merge it. This is why > my SOB is last and not Yishai's. Strictly speaking, the chain should be Yishai->you->Yishai and you'd add your s-o-b again when you pick it. Yeah, that looks like overkill; the current state just looks weird to me, but I'll shut up now.
On Tue, Oct 26, 2021 at 06:32:08PM +0200, Cornelia Huck wrote: > On Tue, Oct 26 2021, Leon Romanovsky <leonro@nvidia.com> wrote: > > > On Tue, Oct 26, 2021 at 05:32:19PM +0200, Cornelia Huck wrote: > >> On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: > >> > >> > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE > >> > instead of STATE). > >> > > >> > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") > >> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> > >> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > >> > >> This s-o-b chain looks weird; your s-o-b always needs to be last. > > > > It is not such clear as it sounds. > > > > Yishai is author of this patch and at some point of time, this patch passed > > through my tree and it will pass again, when we will merge it. This is why > > my SOB is last and not Yishai's. > > Strictly speaking, the chain should be Yishai->you->Yishai and you'd add > your s-o-b again when you pick it. Yeah, that looks like overkill; the > current state just looks weird to me, but I'll shut up now. We will get checkpatch warning about duplicated signature. WARNING: Duplicate signature #11: Signed-off-by: Leon Romanovsky <leonro@nvidia.com> total: 0 errors, 1 warnings, 86 lines checked Thanks >
On Tue, Oct 26 2021, Leon Romanovsky <leonro@nvidia.com> wrote: > On Tue, Oct 26, 2021 at 06:32:08PM +0200, Cornelia Huck wrote: >> On Tue, Oct 26 2021, Leon Romanovsky <leonro@nvidia.com> wrote: >> >> > On Tue, Oct 26, 2021 at 05:32:19PM +0200, Cornelia Huck wrote: >> >> On Tue, Oct 26 2021, Yishai Hadas <yishaih@nvidia.com> wrote: >> >> >> >> > Fixed the non-compiled macro VFIO_DEVICE_STATE_SET_ERROR (i.e. SATE >> >> > instead of STATE). >> >> > >> >> > Fixes: a8a24f3f6e38 ("vfio: UAPI for migration interface for device state") >> >> > Signed-off-by: Yishai Hadas <yishaih@nvidia.com> >> >> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> >> >> >> >> This s-o-b chain looks weird; your s-o-b always needs to be last. >> > >> > It is not such clear as it sounds. >> > >> > Yishai is author of this patch and at some point of time, this patch passed >> > through my tree and it will pass again, when we will merge it. This is why >> > my SOB is last and not Yishai's. >> >> Strictly speaking, the chain should be Yishai->you->Yishai and you'd add >> your s-o-b again when you pick it. Yeah, that looks like overkill; the >> current state just looks weird to me, but I'll shut up now. > > We will get checkpatch warning about duplicated signature. > > WARNING: Duplicate signature > #11: > Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > total: 0 errors, 1 warnings, 86 lines checked ...this looks more like a bug in checkpatch to me, as it is possible for a patch to go through the same person twice. But I'll really shut up now.
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index ef33ea002b0b..114ffcefe437 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -622,7 +622,7 @@ struct vfio_device_migration_info { VFIO_DEVICE_STATE_RESUMING)) #define VFIO_DEVICE_STATE_SET_ERROR(state) \ - ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_SATE_SAVING | \ + ((state & ~VFIO_DEVICE_STATE_MASK) | VFIO_DEVICE_STATE_SAVING | \ VFIO_DEVICE_STATE_RESUMING) __u32 reserved;