diff mbox series

[V4,mlx5-next,06/13] vfio: Fix VFIO_DEVICE_STATE_SET_ERROR macro

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

Commit Message

Yishai Hadas Oct. 26, 2021, 9:05 a.m. UTC
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>
---
 include/uapi/linux/vfio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Cornelia Huck Oct. 26, 2021, 3:32 p.m. UTC | #1
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>
Alex Williamson Oct. 26, 2021, 3:50 p.m. UTC | #2
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
Cornelia Huck Oct. 26, 2021, 3:56 p.m. UTC | #3
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.
Leon Romanovsky Oct. 26, 2021, 4:18 p.m. UTC | #4
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
Cornelia Huck Oct. 26, 2021, 4:32 p.m. UTC | #5
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.
Leon Romanovsky Oct. 26, 2021, 4:42 p.m. UTC | #6
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

>
Cornelia Huck Oct. 26, 2021, 4:57 p.m. UTC | #7
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 mbox series

Patch

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;