Re: [RFC PATCH 0/7] Allow race-free block device handling

On Thu, Feb 02 2023 at  1:41P -0500,
Demi Marie Obenour [off-list ref] wrote:

On Thu, Feb 02, 2023 at 11:50:37AM -0500, Mike Snitzer wrote:

quoted

On Wed, Jan 25 2023 at 10:33P -0500,
Demi Marie Obenour [off-list ref] wrote:

quoted

This work aims to allow userspace to create and destroy block devices
in a race-free and leak-free way,

"race-free and leak-free way" implies there both races and leaks in
existing code. You're making claims that are likely very specific to
your Xen use-case.  Please explain more carefully.

Will do in v2.

quoted

quoted

and to allow them to be exposed to
other Xen VMs via blkback without leaks or races.  It’s marked as RFC
for a few reasons:

- The code has been only lightly tested.  It might be unstable or
  insecure.

- The DM_DEV_CREATE ioctl gains a new flag.  Unknown flags were
  previously ignored, so this could theoretically break buggy userspace
  tools.

Not seeing a reason that type of DM change is needed. If you feel
strongly about it send a separate patch and we can discuss it.

Patch 2/7 is the diskseq change.  v2 will contain a revised and tested
version with a greatly expanded commit message.

I'm aware that 2/7 is where you make the DM change to disallow unknown
flags, what I'm saying is I don't see a reason for that change.

Certainly doesn't look to be a requirement for everything else in that
patch.

So send a separate patch, but I'm inclined to _not_ accept it because
it does potentially break some userspace.
 

quoted

quoted

- I have no idea if I got the block device reference counting and
  locking correct.

Your headers and justifcation for this line of work are really way too
terse. Please take the time to clearly make the case for your changes
in both the patch headers and code.

I will expand the commit message in v2, but I am not sure what you want
me to add to the code comments.  Would you mind explaining?

Nothing specific about code, was just a general reminder (based on how
terse the 2/7 header was).

Mike