[PATCH] virtio-blk: emit udev event when device is resized

[Greg KH]

When virtio-blk device is resized from host (using block_resize from QEMU) emit
KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
custom udev rules which would take whatever action if such event occurs. As a
proof of concept I've created simple udev rule that automatically resize
filesystem on virtio-blk device.
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="ext[3-4]", \
RUN+="/sbin/resize2fs /dev/%k"
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="LVM2_member", \
RUN+="/sbin/pvresize /dev/%k"

This looks fine to me, but I like to check with Greg before adding udev
callouts.... Greg?

Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.

Kay, am I right?

We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

BTW, if this is good, it's good for stable IMHO.

What bug does it fix?

thanks,

greg k-h

Cheers,
Rusty.

---
drivers/block/virtio_blk.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 8ad21a2..5990382 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -539,6 +539,8 @@ static void virtblk_config_changed_work(struct work_struct *work)
struct virtio_device *vdev = vblk->vdev;
struct request_queue *q = vblk->disk->queue;
char cap_str_2[10], cap_str_10[10];
+ char event[] = "RESIZE=1";
+ char *envp[] = { event, NULL };
u64 capacity, size;
mutex_lock(&vblk->config_lock);
@@ -568,6 +570,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
set_capacity(vblk->disk, capacity);
revalidate_disk(vblk->disk);
+ kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
mutex_unlock(&vblk->config_lock);
}
--
1.7.1

[Kay Sievers]

Post by Greg KH
Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?

We removed most of them from the default setups, yes. But there is
nothing wrong if people want to ship that in some package or as custom
rules.

It looks fine to me, we would just not add such things to the default
set of of rules these days.

Post by Greg KH
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

I think we only do that for dm and md and a couple of special cases
like loop and read-only settings.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Greg KH]

Post by Kay Sievers

Post by Greg KH
Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?

We removed most of them from the default setups, yes. But there is
nothing wrong if people want to ship that in some package or as custom
rules.
It looks fine to me, we would just not add such things to the default
set of of rules these days.

Post by Greg KH
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

I think we only do that for dm and md and a couple of special cases
like loop and read-only settings.

What about when we repartition a block device? I've seen the events
happen then.

Anyway, if you are ok with this, no objection from my side then Rusty.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Kay Sievers]

Post by Greg KH

Post by Kay Sievers

Post by Greg KH
Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?

We removed most of them from the default setups, yes. But there is
nothing wrong if people want to ship that in some package or as custom
rules.
It looks fine to me, we would just not add such things to the default
set of of rules these days.

Post by Greg KH
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

I think we only do that for dm and md and a couple of special cases
like loop and read-only settings.

What about when we repartition a block device? I've seen the events
happen then.

Right, from the common block code we send events for removable media
changes like cdroms, sd cards, when a device is switched to read-only,
and when we re-scan a partition table like on re-partitioning. Most of
the other events are block subsystem-specific like this one. For
things like device-mapper they are used pretty heavily.

Post by Greg KH
Anyway, if you are ok with this, no objection from my side then Rusty.

Looks fine to me, it should not do any harm if there are not heavy
programs hooked up -- which is nothing the kernel could fix if people
do that. :)

Kay

[Milos Vyletel]

Post by Kay Sievers

Post by Greg KH
Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?

We removed most of them from the default setups, yes. But there is
nothing wrong if people want to ship that in some package or as custom
rules.
It looks fine to me, we would just not add such things to the default
set of of rules these days.

That was not my intention. I just wanted to demonstrate that event like this
could be useful in cases like filesystem resize. We have a need for automatic
resize by our customers so we will ship our custom udev rules. It's perfectly
understandable that default udev rules will not have this.

Milos

Post by Kay Sievers

Post by Greg KH
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

I think we only do that for dm and md and a couple of special cases
like loop and read-only settings.
Kay

[Milos Vyletel]

Post by Greg KH

When virtio-blk device is resized from host (using block_resize from QEMU) emit
KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
custom udev rules which would take whatever action if such event occurs. As a
proof of concept I've created simple udev rule that automatically resize
filesystem on virtio-blk device.
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="ext[3-4]", \
RUN+="/sbin/resize2fs /dev/%k"
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="LVM2_member", \
RUN+="/sbin/pvresize /dev/%k"

This looks fine to me, but I like to check with Greg before adding udev
callouts.... Greg?

Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

BTW, if this is good, it's good for stable IMHO.

What bug does it fix?

It is not really a bug but it definitely is useful enhancement to have in stable too. I
can imagine lots of people can benefit from this.

Milos

Post by Greg KH
thanks,
greg k-h

Cheers,
Rusty.

---
drivers/block/virtio_blk.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 8ad21a2..5990382 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -539,6 +539,8 @@ static void virtblk_config_changed_work(struct work_struct *work)
struct virtio_device *vdev = vblk->vdev;
struct request_queue *q = vblk->disk->queue;
char cap_str_2[10], cap_str_10[10];
+ char event[] = "RESIZE=1";
+ char *envp[] = { event, NULL };
u64 capacity, size;
mutex_lock(&vblk->config_lock);
@@ -568,6 +570,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
set_capacity(vblk->disk, capacity);
revalidate_disk(vblk->disk);
+ kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
mutex_unlock(&vblk->config_lock);
}
--
1.7.1

[Rusty Russell]

Post by Milos Vyletel

Post by Greg KH

When virtio-blk device is resized from host (using block_resize from QEMU) emit
KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
custom udev rules which would take whatever action if such event occurs. As a
proof of concept I've created simple udev rule that automatically resize
filesystem on virtio-blk device.
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="ext[3-4]", \
RUN+="/sbin/resize2fs /dev/%k"
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="LVM2_member", \
RUN+="/sbin/pvresize /dev/%k"

This looks fine to me, but I like to check with Greg before adding udev
callouts.... Greg?

Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

BTW, if this is good, it's good for stable IMHO.

What bug does it fix?

It is not really a bug but it definitely is useful enhancement to have in stable too. I
can imagine lots of people can benefit from this.

But that applies to almost any enhancement :)

It will go in *next* merge window, not this one.

Thanks,
Rusty.

[Milos Vyletel]

----- Original Message -----

Post by Rusty Russell

Post by Milos Vyletel

Post by Greg KH

When virtio-blk device is resized from host (using block_resize from QEMU) emit
KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
custom udev rules which would take whatever action if such event occurs. As a
proof of concept I've created simple udev rule that automatically resize
filesystem on virtio-blk device.
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="ext[3-4]", \
RUN+="/sbin/resize2fs /dev/%k"
ACTION=="change", KERNEL=="vd*", \
ENV{RESIZE}=="1", \
ENV{ID_FS_TYPE}=="LVM2_member", \
RUN+="/sbin/pvresize /dev/%k"

This looks fine to me, but I like to check with Greg before adding udev
callouts.... Greg?

Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?

BTW, if this is good, it's good for stable IMHO.

What bug does it fix?

It is not really a bug but it definitely is useful enhancement to have in stable too. I
can imagine lots of people can benefit from this.

But that applies to almost any enhancement :)

Good point :)

Post by Rusty Russell
It will go in *next* merge window, not this one.

Cool, thanks.

Milos
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html