virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] vDPA: code clean for vhost_vdpa uapi
  2024-04-10 16:37 [PATCH] vDPA: code clean for vhost_vdpa uapi Zhu Lingshan
@ 2024-04-10  9:27 ` Michael S. Tsirkin
  2024-04-11  2:19   ` Zhu, Lingshan
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-04-10  9:27 UTC (permalink / raw)
  To: Zhu Lingshan; +Cc: sgarzare, jasowang, virtualization

On Thu, Apr 11, 2024 at 12:37:16AM +0800, Zhu Lingshan wrote:
> This commit cleans up the upai for vhost_vdpa by
> better naming some of the macros which report blk
> information to user space.
> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

These are enums not macros.

And tweaking UAPI is problematic as it can break userspace
builds. If you do this you need a #define for the old name
to avoid that issue.

> ---
>  drivers/vdpa/vdpa.c       | 6 +++---
>  include/uapi/linux/vdpa.h | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index b246067e074b..6cb96a1e8b7d 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
>  
>  	val_u32 = __virtio32_to_cpu(true, config->size_max);
>  
> -	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
> +	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
>  }
>  
>  /* fill the block size*/
> @@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
>  	u8 ro;
>  
>  	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
>  		return -EMSGSIZE;
>  
>  	return 0;
> @@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
>  	u8 flush;
>  
>  	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
>  		return -EMSGSIZE;
>  
>  	return 0;
> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> index 43c51698195c..842bf1201ac4 100644
> --- a/include/uapi/linux/vdpa.h
> +++ b/include/uapi/linux/vdpa.h
> @@ -57,7 +57,7 @@ enum vdpa_attr {
>  	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
>  
>  	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
> -	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
> +	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
>  	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
>  	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
>  	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
> @@ -70,8 +70,8 @@ enum vdpa_attr {
>  	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
>  	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
>  	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
> -	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
> -	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
> +	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
> +	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
>  
>  	/* new attributes must be added above here */
>  	VDPA_ATTR_MAX,
> -- 
> 2.39.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] vDPA: code clean for vhost_vdpa uapi
@ 2024-04-10 16:37 Zhu Lingshan
  2024-04-10  9:27 ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Zhu Lingshan @ 2024-04-10 16:37 UTC (permalink / raw)
  To: sgarzare, jasowang, mst; +Cc: virtualization, Zhu Lingshan

This commit cleans up the upai for vhost_vdpa by
better naming some of the macros which report blk
information to user space.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/vdpa.c       | 6 +++---
 include/uapi/linux/vdpa.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index b246067e074b..6cb96a1e8b7d 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
 
 	val_u32 = __virtio32_to_cpu(true, config->size_max);
 
-	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
+	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
 }
 
 /* fill the block size*/
@@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
 	u8 ro;
 
 	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
-	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
+	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
 		return -EMSGSIZE;
 
 	return 0;
@@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
 	u8 flush;
 
 	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
-	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
+	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
 		return -EMSGSIZE;
 
 	return 0;
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 43c51698195c..842bf1201ac4 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -57,7 +57,7 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
 
 	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
-	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
+	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
 	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
 	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
 	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
@@ -70,8 +70,8 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
 	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
 	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
-	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
-	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
+	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
+	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
 
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] vDPA: code clean for vhost_vdpa uapi
  2024-04-10  9:27 ` Michael S. Tsirkin
@ 2024-04-11  2:19   ` Zhu, Lingshan
  2024-04-11  6:05     ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Zhu, Lingshan @ 2024-04-11  2:19 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: sgarzare, jasowang, virtualization



On 4/10/2024 5:27 PM, Michael S. Tsirkin wrote:
> On Thu, Apr 11, 2024 at 12:37:16AM +0800, Zhu Lingshan wrote:
>> This commit cleans up the upai for vhost_vdpa by
>> better naming some of the macros which report blk
>> information to user space.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> These are enums not macros.
>
> And tweaking UAPI is problematic as it can break userspace
> builds. If you do this you need a #define for the old name
> to avoid that issue.
The enums are not in 6.9 official release yet, so we have a chance and 
agreed
to deliver a fix during RC stage.

https://lore.kernel.org/all/20240331160230-mutt-send-email-mst@kernel.org/

Shall I post a V2 for better commit message: macros --> enums?

Thanks
Zhu Lingshan
>
>> ---
>>   drivers/vdpa/vdpa.c       | 6 +++---
>>   include/uapi/linux/vdpa.h | 6 +++---
>>   2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>> index b246067e074b..6cb96a1e8b7d 100644
>> --- a/drivers/vdpa/vdpa.c
>> +++ b/drivers/vdpa/vdpa.c
>> @@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
>>   
>>   	val_u32 = __virtio32_to_cpu(true, config->size_max);
>>   
>> -	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
>> +	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
>>   }
>>   
>>   /* fill the block size*/
>> @@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
>>   	u8 ro;
>>   
>>   	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
>> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
>> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
>>   		return -EMSGSIZE;
>>   
>>   	return 0;
>> @@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
>>   	u8 flush;
>>   
>>   	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
>> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
>> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
>>   		return -EMSGSIZE;
>>   
>>   	return 0;
>> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
>> index 43c51698195c..842bf1201ac4 100644
>> --- a/include/uapi/linux/vdpa.h
>> +++ b/include/uapi/linux/vdpa.h
>> @@ -57,7 +57,7 @@ enum vdpa_attr {
>>   	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
>>   
>>   	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
>> -	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
>> +	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
>>   	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
>>   	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
>>   	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
>> @@ -70,8 +70,8 @@ enum vdpa_attr {
>>   	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
>>   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
>>   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
>> -	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
>> -	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
>> +	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
>> +	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
>>   
>>   	/* new attributes must be added above here */
>>   	VDPA_ATTR_MAX,
>> -- 
>> 2.39.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vDPA: code clean for vhost_vdpa uapi
  2024-04-11  2:19   ` Zhu, Lingshan
@ 2024-04-11  6:05     ` Michael S. Tsirkin
  2024-04-14  8:28       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-04-11  6:05 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: sgarzare, jasowang, virtualization

On Thu, Apr 11, 2024 at 10:19:38AM +0800, Zhu, Lingshan wrote:
> 
> 
> On 4/10/2024 5:27 PM, Michael S. Tsirkin wrote:
> > On Thu, Apr 11, 2024 at 12:37:16AM +0800, Zhu Lingshan wrote:
> > > This commit cleans up the upai for vhost_vdpa by
> > > better naming some of the macros which report blk
> > > information to user space.
> > > 
> > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> > These are enums not macros.
> > 
> > And tweaking UAPI is problematic as it can break userspace
> > builds. If you do this you need a #define for the old name
> > to avoid that issue.
> The enums are not in 6.9 official release yet, so we have a chance and
> agreed
> to deliver a fix during RC stage.
> 
> https://lore.kernel.org/all/20240331160230-mutt-send-email-mst@kernel.org/
> 
> Shall I post a V2 for better commit message: macros --> enums?
> 
> Thanks
> Zhu Lingshan


Pls do. Add a Fixes: tag listing the commits that introduced them.
And mention that it wasn't in any releases.

> > 
> > > ---
> > >   drivers/vdpa/vdpa.c       | 6 +++---
> > >   include/uapi/linux/vdpa.h | 6 +++---
> > >   2 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > index b246067e074b..6cb96a1e8b7d 100644
> > > --- a/drivers/vdpa/vdpa.c
> > > +++ b/drivers/vdpa/vdpa.c
> > > @@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
> > >   	val_u32 = __virtio32_to_cpu(true, config->size_max);
> > > -	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
> > > +	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
> > >   }
> > >   /* fill the block size*/
> > > @@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
> > >   	u8 ro;
> > >   	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
> > > -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
> > > +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
> > >   		return -EMSGSIZE;
> > >   	return 0;
> > > @@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
> > >   	u8 flush;
> > >   	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
> > > -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
> > > +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
> > >   		return -EMSGSIZE;
> > >   	return 0;
> > > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> > > index 43c51698195c..842bf1201ac4 100644
> > > --- a/include/uapi/linux/vdpa.h
> > > +++ b/include/uapi/linux/vdpa.h
> > > @@ -57,7 +57,7 @@ enum vdpa_attr {
> > >   	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
> > > -	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
> > > +	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
> > > @@ -70,8 +70,8 @@ enum vdpa_attr {
> > >   	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
> > >   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
> > > -	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
> > > -	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
> > > +	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
> > > +	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
> > >   	/* new attributes must be added above here */
> > >   	VDPA_ATTR_MAX,
> > > -- 
> > > 2.39.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vDPA: code clean for vhost_vdpa uapi
  2024-04-11  6:05     ` Michael S. Tsirkin
@ 2024-04-14  8:28       ` Michael S. Tsirkin
  2024-04-15  2:46         ` Zhu, Lingshan
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-04-14  8:28 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: sgarzare, jasowang, virtualization

On Thu, Apr 11, 2024 at 02:05:57AM -0400, Michael S. Tsirkin wrote:
> On Thu, Apr 11, 2024 at 10:19:38AM +0800, Zhu, Lingshan wrote:
> > 
> > 
> > On 4/10/2024 5:27 PM, Michael S. Tsirkin wrote:
> > > On Thu, Apr 11, 2024 at 12:37:16AM +0800, Zhu Lingshan wrote:
> > > > This commit cleans up the upai for vhost_vdpa by
> > > > better naming some of the macros which report blk
> > > > information to user space.
> > > > 
> > > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> > > These are enums not macros.
> > > 
> > > And tweaking UAPI is problematic as it can break userspace
> > > builds. If you do this you need a #define for the old name
> > > to avoid that issue.
> > The enums are not in 6.9 official release yet, so we have a chance and
> > agreed
> > to deliver a fix during RC stage.
> > 
> > https://lore.kernel.org/all/20240331160230-mutt-send-email-mst@kernel.org/
> > 
> > Shall I post a V2 for better commit message: macros --> enums?
> > 
> > Thanks
> > Zhu Lingshan
> 
> 
> Pls do. Add a Fixes: tag listing the commits that introduced them.
> And mention that it wasn't in any releases.


Still waiting for this. Note I keep changes in next for several days
so there's a chance to shake out bugs, and if I send a pull
at the last minute it might not make it in the release.
We'll then not be able to make these changes to avoid breaking
userspace builds.


> > > 
> > > > ---
> > > >   drivers/vdpa/vdpa.c       | 6 +++---
> > > >   include/uapi/linux/vdpa.h | 6 +++---
> > > >   2 files changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > > index b246067e074b..6cb96a1e8b7d 100644
> > > > --- a/drivers/vdpa/vdpa.c
> > > > +++ b/drivers/vdpa/vdpa.c
> > > > @@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
> > > >   	val_u32 = __virtio32_to_cpu(true, config->size_max);
> > > > -	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
> > > > +	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
> > > >   }
> > > >   /* fill the block size*/
> > > > @@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
> > > >   	u8 ro;
> > > >   	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
> > > > -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
> > > > +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
> > > >   		return -EMSGSIZE;
> > > >   	return 0;
> > > > @@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
> > > >   	u8 flush;
> > > >   	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
> > > > -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
> > > > +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
> > > >   		return -EMSGSIZE;
> > > >   	return 0;
> > > > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> > > > index 43c51698195c..842bf1201ac4 100644
> > > > --- a/include/uapi/linux/vdpa.h
> > > > +++ b/include/uapi/linux/vdpa.h
> > > > @@ -57,7 +57,7 @@ enum vdpa_attr {
> > > >   	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
> > > > -	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
> > > > +	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
> > > > @@ -70,8 +70,8 @@ enum vdpa_attr {
> > > >   	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
> > > >   	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
> > > > -	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
> > > > -	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
> > > > +	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
> > > > +	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
> > > >   	/* new attributes must be added above here */
> > > >   	VDPA_ATTR_MAX,
> > > > -- 
> > > > 2.39.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] vDPA: code clean for vhost_vdpa uapi
  2024-04-14  8:28       ` Michael S. Tsirkin
@ 2024-04-15  2:46         ` Zhu, Lingshan
  0 siblings, 0 replies; 6+ messages in thread
From: Zhu, Lingshan @ 2024-04-15  2:46 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: sgarzare, jasowang, virtualization



On 4/14/2024 4:28 PM, Michael S. Tsirkin wrote:
> On Thu, Apr 11, 2024 at 02:05:57AM -0400, Michael S. Tsirkin wrote:
>> On Thu, Apr 11, 2024 at 10:19:38AM +0800, Zhu, Lingshan wrote:
>>>
>>> On 4/10/2024 5:27 PM, Michael S. Tsirkin wrote:
>>>> On Thu, Apr 11, 2024 at 12:37:16AM +0800, Zhu Lingshan wrote:
>>>>> This commit cleans up the upai for vhost_vdpa by
>>>>> better naming some of the macros which report blk
>>>>> information to user space.
>>>>>
>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>> These are enums not macros.
>>>>
>>>> And tweaking UAPI is problematic as it can break userspace
>>>> builds. If you do this you need a #define for the old name
>>>> to avoid that issue.
>>> The enums are not in 6.9 official release yet, so we have a chance and
>>> agreed
>>> to deliver a fix during RC stage.
>>>
>>> https://lore.kernel.org/all/20240331160230-mutt-send-email-mst@kernel.org/
>>>
>>> Shall I post a V2 for better commit message: macros --> enums?
>>>
>>> Thanks
>>> Zhu Lingshan
>>
>> Pls do. Add a Fixes: tag listing the commits that introduced them.
>> And mention that it wasn't in any releases.
>
> Still waiting for this. Note I keep changes in next for several days
> so there's a chance to shake out bugs, and if I send a pull
> at the last minute it might not make it in the release.
> We'll then not be able to make these changes to avoid breaking
> userspace builds.
I was on vocations for my personal affairs and will deliver a V2 patch
today.

I see you have send out a pull request, maybe we still have
a chance to fix it in RC5?

Thanks
Zhu Lingshan
>
>
>>>>> ---
>>>>>    drivers/vdpa/vdpa.c       | 6 +++---
>>>>>    include/uapi/linux/vdpa.h | 6 +++---
>>>>>    2 files changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>>>>> index b246067e074b..6cb96a1e8b7d 100644
>>>>> --- a/drivers/vdpa/vdpa.c
>>>>> +++ b/drivers/vdpa/vdpa.c
>>>>> @@ -967,7 +967,7 @@ vdpa_dev_blk_seg_size_config_fill(struct sk_buff *msg, u64 features,
>>>>>    	val_u32 = __virtio32_to_cpu(true, config->size_max);
>>>>> -	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE, val_u32);
>>>>> +	return nla_put_u32(msg, VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX, val_u32);
>>>>>    }
>>>>>    /* fill the block size*/
>>>>> @@ -1089,7 +1089,7 @@ static int vdpa_dev_blk_ro_config_fill(struct sk_buff *msg, u64 features)
>>>>>    	u8 ro;
>>>>>    	ro = ((features & BIT_ULL(VIRTIO_BLK_F_RO)) == 0) ? 0 : 1;
>>>>> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_READ_ONLY, ro))
>>>>> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_READ_ONLY, ro))
>>>>>    		return -EMSGSIZE;
>>>>>    	return 0;
>>>>> @@ -1100,7 +1100,7 @@ static int vdpa_dev_blk_flush_config_fill(struct sk_buff *msg, u64 features)
>>>>>    	u8 flush;
>>>>>    	flush = ((features & BIT_ULL(VIRTIO_BLK_F_FLUSH)) == 0) ? 0 : 1;
>>>>> -	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_CFG_FLUSH, flush))
>>>>> +	if (nla_put_u8(msg, VDPA_ATTR_DEV_BLK_FLUSH, flush))
>>>>>    		return -EMSGSIZE;
>>>>>    	return 0;
>>>>> diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
>>>>> index 43c51698195c..842bf1201ac4 100644
>>>>> --- a/include/uapi/linux/vdpa.h
>>>>> +++ b/include/uapi/linux/vdpa.h
>>>>> @@ -57,7 +57,7 @@ enum vdpa_attr {
>>>>>    	VDPA_ATTR_DEV_FEATURES,                 /* u64 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_CAPACITY,		/* u64 */
>>>>> -	VDPA_ATTR_DEV_BLK_CFG_SEG_SIZE,		/* u32 */
>>>>> +	VDPA_ATTR_DEV_BLK_CFG_SIZE_MAX,		/* u32 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_BLK_SIZE,		/* u32 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_SEG_MAX,		/* u32 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_NUM_QUEUES,	/* u16 */
>>>>> @@ -70,8 +70,8 @@ enum vdpa_attr {
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_DISCARD_SEC_ALIGN,/* u32 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEC,	/* u32 */
>>>>>    	VDPA_ATTR_DEV_BLK_CFG_MAX_WRITE_ZEROES_SEG,	/* u32 */
>>>>> -	VDPA_ATTR_DEV_BLK_CFG_READ_ONLY,		/* u8 */
>>>>> -	VDPA_ATTR_DEV_BLK_CFG_FLUSH,		/* u8 */
>>>>> +	VDPA_ATTR_DEV_BLK_READ_ONLY,		/* u8 */
>>>>> +	VDPA_ATTR_DEV_BLK_FLUSH,		/* u8 */
>>>>>    	/* new attributes must be added above here */
>>>>>    	VDPA_ATTR_MAX,
>>>>> -- 
>>>>> 2.39.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-04-15  2:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 16:37 [PATCH] vDPA: code clean for vhost_vdpa uapi Zhu Lingshan
2024-04-10  9:27 ` Michael S. Tsirkin
2024-04-11  2:19   ` Zhu, Lingshan
2024-04-11  6:05     ` Michael S. Tsirkin
2024-04-14  8:28       ` Michael S. Tsirkin
2024-04-15  2:46         ` Zhu, Lingshan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).