Wed, Apr 29, 2020 at 06:35:18PM CEST, k...@kernel.org wrote:
>On Wed, 29 Apr 2020 07:45:52 +0200 Jiri Pirko wrote:
>> Wed, Apr 29, 2020 at 03:42:48AM CEST, k...@kernel.org wrote:
>> >Jiri, this is what I had in mind of snapshots and the same
>> >thing will come back for slice allocation.
>>
>> Okay. Could you please send the userspace patch too in order to see the
>> full picture?
>
>You got it, I didn't do anything fancy there.
>
>> >+static int
>> >+devlink_nl_alloc_snapshot_id(struct devlink *devlink, struct genl_info
>> >*info,
>> >+ struct devlink_region *region, u32 *snapshot_id)
>> >+{
>> >+ struct sk_buff *msg;
>> >+ void *hdr;
>> >+ int err;
>> >+
>> >+ err = __devlink_region_snapshot_id_get(devlink, snapshot_id);
>> >+ if (err) {
>> >+ NL_SET_ERR_MSG_MOD(info->extack,
>>
>> No need to wrap here.
>
>Ok.
>
>> >+ "Failed to allocate a new snapshot id");
>> >+ return err;
>> >+ }
>
>> >- err = __devlink_snapshot_id_insert(devlink, snapshot_id);
>> >- if (err)
>> >- return err;
>> >+ err = __devlink_snapshot_id_insert(devlink, snapshot_id);
>> >+ if (err)
>> >+ return err;
>> >+ } else {
>> >+ err = devlink_nl_alloc_snapshot_id(devlink, info,
>> >+ region, &snapshot_id);
>> >+ if (err)
>> >+ return err;
>> >+ }
>> >
>> > err = region->ops->snapshot(devlink, info->extack, &data);
>>
>> How the output is going to looks like it this or any of the follow-up
>> calls in this function are going to fail?
>>
>> I guess it is going to be handled gracefully in the userspace app,
>> right?
>
>The output is the same, just the return code is non-zero.
>
>I can change that not to print the snapshot info until we are sure the
>operation succeeded if you prefer.
I think that would be clean to do this, in kernel.
>
>Initially I had the kernel not sent the message until it's done with
>the operation, but that seems unnecessarily complex. The send operation
>itself may fail, and if we ever have an operation that requires more
>than one notification we'll have to struggle with atomic sends.
>
>Better have the user space treat the failure like an exception, and
>ignore all the notifications that came earlier.
>
>That said the iproute2 patch can be improved with extra 20 lines so
>that it holds off printing the snapshot info until success.
