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@xxxxxxx>
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
