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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux