diff mbox

[OpenWrt-Devel] procd: hotplug.json: allow passing hotplug events from all subsystems

Message ID 1448684520-4485-1-git-send-email-yszhou4tech@gmail.com
State Rejected
Headers show

Commit Message

Yousong Zhou Nov. 28, 2015, 4:22 a.m. UTC
There are time that programs need to be notified of events from
subsystems that are not enumerated in the .json definition, e.g. QEMU
guest agent by default requires /dev/virtio-ports/org.qemu.guest_agent.0
which is a symlink to /dev/vportMpN from virtio-ports subsystem.

Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
---
 package/system/procd/files/hotplug.json | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Yousong Zhou Nov. 28, 2015, 4:28 a.m. UTC | #1
On 28 November 2015 at 12:22, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> There are time that programs need to be notified of events from
> subsystems that are not enumerated in the .json definition, e.g. QEMU
> guest agent by default requires /dev/virtio-ports/org.qemu.guest_agent.0
> which is a symlink to /dev/vportMpN from virtio-ports subsystem.
>

For the record, the PR about qemu-ga is at
https://github.com/openwrt/packages/pull/2041

                yousong

> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
> ---
>  package/system/procd/files/hotplug.json | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/package/system/procd/files/hotplug.json b/package/system/procd/files/hotplug.json
> index 27b4836..bad2340 100644
> --- a/package/system/procd/files/hotplug.json
> +++ b/package/system/procd/files/hotplug.json
> @@ -69,18 +69,13 @@
>                 [ "button", "/etc/rc.button/%BUTTON%" ]
>         ],
>         [ "if",
> -               [ "eq", "SUBSYSTEM",
> -                       [ "net", "input", "usb", "usbmisc", "ieee1394", "block", "atm", "zaptel", "tty", "button" ]
> -               ],
> -               [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
> -       ],
> -       [ "if",
>                 [ "and",
>                         [ "eq", "SUBSYSTEM", "usb-serial" ],
>                         [ "regex", "DEVNAME",
>                                 [ "^ttyUSB", "^ttyACM" ]
>                         ],
>                 ],
> -               [ "exec", "/sbin/hotplug-call", "tty" ]
> +               [ "exec", "/sbin/hotplug-call", "tty" ],
> +               [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
>         ],
>  ]
> --
> 2.6.3
>
John Crispin Nov. 28, 2015, 11:13 p.m. UTC | #2
Hi Yousong,

On 28/11/2015 05:22, Yousong Zhou wrote:
> There are time that programs need to be notified of events from
> subsystems that are not enumerated in the .json definition, e.g. QEMU
> guest agent by default requires /dev/virtio-ports/org.qemu.guest_agent.0
> which is a symlink to /dev/vportMpN from virtio-ports subsystem.
> 

i am not sure if this is a good idea. there are thousands of events
being broadcast, specially during boot and we really want to avoid
respawning the script helper for each one of them. i was under the
impression that we had an include directive that allowed us to include
board/target specific json files. however i am failing to find the code
that does this so the feature might not actually be implemented yet.
i'll need to have a closer look at this the next days.

	John

> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
> ---
>  package/system/procd/files/hotplug.json | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/package/system/procd/files/hotplug.json b/package/system/procd/files/hotplug.json
> index 27b4836..bad2340 100644
> --- a/package/system/procd/files/hotplug.json
> +++ b/package/system/procd/files/hotplug.json
> @@ -69,18 +69,13 @@
>  		[ "button", "/etc/rc.button/%BUTTON%" ]
>  	],
>  	[ "if",
> -		[ "eq", "SUBSYSTEM",
> -			[ "net", "input", "usb", "usbmisc", "ieee1394", "block", "atm", "zaptel", "tty", "button" ]
> -		],
> -		[ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
> -	],
> -	[ "if",
>  		[ "and",
>  			[ "eq", "SUBSYSTEM", "usb-serial" ],
>  			[ "regex", "DEVNAME",
>  				[ "^ttyUSB", "^ttyACM" ]
>  			],
>  		],
> -		[ "exec", "/sbin/hotplug-call", "tty" ]
> +		[ "exec", "/sbin/hotplug-call", "tty" ],
> +		[ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
>  	],
>  ]
>
Luiz Angelo Daros de Luca Nov. 29, 2015, 4:10 a.m. UTC | #3
John,

How about generate the subsystem list from /etc/hotplug.d dirs. The list
could be loaded at startup and refreshed the same way hotplug.json is (
which might be "never" as procd does not have an "init q").

Regards,

Luiz

Em sáb, 28 de nov de 2015 21:14, John Crispin <blogic@openwrt.org> escreveu:

> Hi Yousong,
>
> On 28/11/2015 05:22, Yousong Zhou wrote:
> > There are time that programs need to be notified of events from
> > subsystems that are not enumerated in the .json definition, e.g. QEMU
> > guest agent by default requires /dev/virtio-ports/org.qemu.guest_agent.0
> > which is a symlink to /dev/vportMpN from virtio-ports subsystem.
> >
>
> i am not sure if this is a good idea. there are thousands of events
> being broadcast, specially during boot and we really want to avoid
> respawning the script helper for each one of them. i was under the
> impression that we had an include directive that allowed us to include
> board/target specific json files. however i am failing to find the code
> that does this so the feature might not actually be implemented yet.
> i'll need to have a closer look at this the next days.
>
>         John
>
> > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com>
> > ---
> >  package/system/procd/files/hotplug.json | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/package/system/procd/files/hotplug.json
> b/package/system/procd/files/hotplug.json
> > index 27b4836..bad2340 100644
> > --- a/package/system/procd/files/hotplug.json
> > +++ b/package/system/procd/files/hotplug.json
> > @@ -69,18 +69,13 @@
> >               [ "button", "/etc/rc.button/%BUTTON%" ]
> >       ],
> >       [ "if",
> > -             [ "eq", "SUBSYSTEM",
> > -                     [ "net", "input", "usb", "usbmisc", "ieee1394",
> "block", "atm", "zaptel", "tty", "button" ]
> > -             ],
> > -             [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
> > -     ],
> > -     [ "if",
> >               [ "and",
> >                       [ "eq", "SUBSYSTEM", "usb-serial" ],
> >                       [ "regex", "DEVNAME",
> >                               [ "^ttyUSB", "^ttyACM" ]
> >                       ],
> >               ],
> > -             [ "exec", "/sbin/hotplug-call", "tty" ]
> > +             [ "exec", "/sbin/hotplug-call", "tty" ],
> > +             [ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
> >       ],
> >  ]
> >
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>
John Crispin Nov. 29, 2015, 10:26 a.m. UTC | #4
On 29/11/2015 05:10, Luiz Angelo Daros de Luca wrote:
> John,
> 
> How about generate the subsystem list from /etc/hotplug.d dirs. The list
> could be loaded at startup and refreshed the same way hotplug.json is (
> which might be "never" as procd does not have an "init q").
> 
> Regards,
> 
> Luiz
> 

Hi,

nope that would not go well with our dynamic config reload as it would
be boot time config. additionally the system comes up on a r/o fs during
preinit so you would not be able to fiddle with the files anyhow. all
changes you make wont be available that early during boot.

John
Jo-Philipp Wich Nov. 30, 2015, 9:33 a.m. UTC | #5
Hi all,

as an intermediate solution we could simply allow qemu specific events.

I'm with John here that passing on uevents unfiltered is probably no
good idea.

~ Jow
John Crispin Nov. 30, 2015, 9:50 a.m. UTC | #6
On 30/11/2015 10:33, Jo-Philipp Wich wrote:
> Hi all,
> 
> as an intermediate solution we could simply allow qemu specific events.
> 
> I'm with John here that passing on uevents unfiltered is probably no
> good idea.
> 
> ~ Jow

yep, hence my idea about includes

we could ship a "include /etc/hotplug.arch/*" and then any target can
just place its custom files there. they would be available prior to
overlay being available and wont clutter the main json file.

	John


> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>
Yousong Zhou Nov. 30, 2015, 9:58 a.m. UTC | #7
On 30 November 2015 at 17:50, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 30/11/2015 10:33, Jo-Philipp Wich wrote:
>> Hi all,
>>
>> as an intermediate solution we could simply allow qemu specific events.
>>
>> I'm with John here that passing on uevents unfiltered is probably no
>> good idea.
>>
>> ~ Jow
>
> yep, hence my idea about includes
>
> we could ship a "include /etc/hotplug.arch/*" and then any target can
> just place its custom files there. they would be available prior to
> overlay being available and wont clutter the main json file.
>
>         John
>

I just noticed that handle_expr callback of json_script_ctx was never
actually used.  In the specific case of hotplug events handling of
procd, we can just add a 'isdir' '/etc/hotplug.d/%SUBSYSTEM%' expr and
do the hotplug-call accordingly.  Well, now this needs to fix libubox
first so cc-ing Felix.

                yousong
diff mbox

Patch

diff --git a/package/system/procd/files/hotplug.json b/package/system/procd/files/hotplug.json
index 27b4836..bad2340 100644
--- a/package/system/procd/files/hotplug.json
+++ b/package/system/procd/files/hotplug.json
@@ -69,18 +69,13 @@ 
 		[ "button", "/etc/rc.button/%BUTTON%" ]
 	],
 	[ "if",
-		[ "eq", "SUBSYSTEM",
-			[ "net", "input", "usb", "usbmisc", "ieee1394", "block", "atm", "zaptel", "tty", "button" ]
-		],
-		[ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
-	],
-	[ "if",
 		[ "and",
 			[ "eq", "SUBSYSTEM", "usb-serial" ],
 			[ "regex", "DEVNAME",
 				[ "^ttyUSB", "^ttyACM" ]
 			],
 		],
-		[ "exec", "/sbin/hotplug-call", "tty" ]
+		[ "exec", "/sbin/hotplug-call", "tty" ],
+		[ "exec", "/sbin/hotplug-call", "%SUBSYSTEM%" ]
 	],
 ]