Re: [PATCH 30/45] block: remove the nr_sects field in struct hd_struct

Jan Kara
2020-11-30T15:18:25Z

On Mon 30-11-20 15:51:50, Christoph Hellwig wrote:
> On Mon, Nov 30, 2020 at 10:44:21AM +0100, Jan Kara wrote:
> > I know I'm like a broken record about this but I still don't understand
> > here... I'd expect the new code to be:
> >
> > if (size == capacity ||
> > (disk->flags & (GENHD_FL_UP | GENHD_FL_HIDDEN)) != GENHD_FL_UP)
> > return false;
> > pr_info("%s: detected capacity change from %lld to %lld\n",
> > disk->disk_name, size, capacity);
> > + if (!capacity || !size)
> > + return false;
> > kobject_uevent_env(&disk_to_dev(disk)->kobj, KOBJ_CHANGE, envp);
> > return true;
> >
> > At least that would be equivalent to the original functionality of
> > set_capacity_and_notify(). And if you indeed intend to change when
> > "RESIZE=1" events are sent, then I'd expect an explanation in the changelog
> > why this cannot break userspace (I remember we've already broken some udev
> > rules in the past by generating unexpected events and we had to revert
> > those changes in the partition code so I'm more careful now). The rest of
> > the patch looks good to me.
>
> I explained that I think the GENHD_FL_UP is the more useful one here in
> reply to your last comment. If the size changes to or from 0 during
> runtime we probably do want an event.

Ah, right, sorry, I missed that. And I agree that it might make sense for
changes to / from zero during runtime to send notification. But it still
seems as a thin ice with potential to breakage to me.

> But I'll add your hunk for now and we can discuss this separately.

OK, thanks. With that hunk feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

--
Jan Kara <jack@suse.com>
SUSE Labs, CR