On Wed, Aug 07, 2024 at 12:07:26 +0800, Chun Feng Wu wrote: Please do not top-post on technical lists. Put your reply inline with the relevant context. > let me confirm my understanding, do you mean there should be group name > mapping between DOM($group_name_in_DOM) and QOM(throttle-$group_name_in_DOM) > for both throttle group and throttle filter? if so, there seems two ways to > achieve that: The main point is that the throttle group names are arbitrary, we must ensure that the user won't pick a name that collides with any other QOM name or for that matter for any thing else. There are multiple ways how to achieve that, but the easiest seems to add a prefix and sanitize the name to e.g contain only characters and numbers. Others like numbering them arbitrarily or so seem to be too much hassle. For the -blockdev throttle naming they should use existing node name generators and not be based on anything user provided. Node-names are length-limited so users might pick a name that'd exceed the limit. Thus not allowing any control is the best there. > - mapping group_name in callers like qemu_driver.c, qemu_hotplug.c, > qemu_command.c > > - or put all mappings only into qemu_monitor.c/qemu_monitor_json.c, in this > way, I may need to expose more methods within monitor to prepare > virJSONValueObject for ThrottleGroup and ThrottleFilter creation, it seems > this way can centralize mapping logic within monitor only It can't be limited to monitor, because you need to construct the trhottling '-object' also when starting up qemu. So it needs to apply wherever appropriate. As the prefix is constant you don't need any helpers or extra infra to decorate it. Just add it where appropriate. > > On 2024/7/26 21:58, Peter Krempa wrote: > > Note that since all objects live in one namespace in qemu you'll have to > > add a prefix to the group name so that the user will not be able to > > accidentaly pick a group name (which would be equivalent with the object > > 'id') which we might either be using or introduce in the future. > > > > Add a 'throttle-' prefix so that we clearly separate the throttle group > > objects into their own namespace. > > > > This will need to be done everywhere where you pass the throttle group > > name as object name to qemu. > > -- > Thanks and Regards, > > Wu
