On 08/17/2016 04:43 AM, Lukáš Doktor wrote: > Hello guys, >
Hi Lukáš, Here are a few comments/questions that arose from a quick read. > While looking at the > https://github.com/avocado-framework/avocado-virt/pull/90 I noticed > quite serious issue in the current multiplexer design and if I > understand it correctly even in the ideal multiplexer design (avocado > still does not support the full specification). I'm not sure whether I > just don't see something obvious so please take a look at it. > > The problem is, that in that PR Fam assigns the value of non-leaf item. > That works well, the problem is when one tries to multiplex it, because > he changes the non-leaf object to !mux and it adds the other branches as > different mux domains. > > Let's make it concrete. The idea is to expect `kvm = on|off` in > `/plugins/virt/qemu/*` path. This path contains several children already > (`/plugins/virt/qemu/paths` and `/plugins/virt/qemu/migrate`), which > still works well (one can generate similar output with `avocado > multiplex --system-wide ...` when avocado-virt is installed; note you > can't use `-s` as it clashes with `--silent`, PR to fix it has been sent): > > ``` > ┗━━ plugins > ┗━━ virt > ┣━━ qemu > ┃ ┣━━ paths > ┃ ┃ → qemu_dst_bin: None > ┃ ┃ → qemu_img_bin: None > ┃ ┃ → kvm: on > ┃ ┃ → qemu_bin: None > ┃ ┗━━ migrate > ┃ → kvm: on > ┃ → timeout: 60.0 > ``` > > Now when you want to vary between different values, you might want to > inject something like: > > ``` > plugins: > virt: > qemu: !mux > enabled: > kvm: on > disabled: > qemu: off > ``` You mean "kvm: off" here, right? > > The problem is, that it affects the whole `/plugins/virt/qemu` node and > all it's children become multiplexed: > > ``` > ┗━━ plugins > ┗━━ virt > ┣━━ qemu > ┃ ╠══ enabled > ┃ ║ → kvm: on > ┃ ╠══ disabled > ┃ ║ → kvm: off > ┃ ╠══ paths > ┃ ║ → qemu_dst_bin: None > ┃ ║ → qemu_img_bin: None > ┃ ║ → qemu_bin: None > ┃ ╚══ migrate > ┃ → timeout: 60.0 > ``` > > The original multiplexer RFC wanted to always allow left side to be > optional. This currently works only for paths inside mux-paths (like > /run). Anyway let's assume it works and explore the consequences. The > query path would change to `*/plugins/virt/qemu/*` and we'd inject the > values on a different place: Do you mean injecting nodes with a non-absolute (left-most part missing) path? I fail to see how that would be possible. > > ``` > ┣━━ my > ┃ ┗━━ kvm > ┃ ┗━━ plugins > ┃ ┗━━ virt > ┃ ┗━━ qemu > ┃ ╠══ enabled > ┃ ║ → kvm: on > ┃ ╚══ disabled > ┃ → kvm: off > ┗━━ plugins > ┗━━ virt > ┣━━ qemu > ┃ ┣━━ paths > ┃ ┃ → qemu_dst_bin: None > ┃ ┃ → qemu_img_bin: None > ┃ ┃ → qemu_bin: None > ┃ ┗━━ migrate > ┃ → timeout: 60.0 > ``` > > 1. mux looks into mux-path (`/run`) first, can't find the > `kvm/plugins/virt/qemu` there > 2. it looks into `/` and finds it in `/my/kvm/plugins/virt/qemu` -> success > > The problem is, when we use defaults (eg. the way we define default > values in avocado-virt): > > ``` > ┣━━ my > ┃ ┗━━ kvm > ┃ ┗━━ plugins > ┃ ┗━━ virt > ┃ ┗━━ qemu > ┃ ╠══ enabled > ┃ ║ → kvm: on > ┃ ╚══ disabled > ┃ → kvm: off > ┗━━ plugins > ┗━━ virt > ┣━━ qemu > ┃ ┣━━ paths > ┃ ┃ → qemu_dst_bin: None > ┃ ┃ → qemu_img_bin: None > ┃ ┃ → kvm: Something_else > ┃ ┃ → qemu_bin: None > ┃ ┗━━ migrate > ┃ → kvm: Something_else > ┃ → timeout: 60.0 > ``` > > 1. mux looks into mux-path (`/run`) first, can't find the > `kvm/plugins/virt/qemu` there > 2. it looks into `/` and finds it in `/my/kvm/plugins/virt/qemu` and in > `/plugins/virt/qemu` -> failure > > Yes, one could solve it by defining another `mux-path` to `/my` or even > `/my/kvm`, but that just adds the complexity. > > Let me also mention why do we like to extend nodes from right. Imagine > we expect `disk_type` in `/virt/hw/disk/*`. The yaml file might look > like this: > > ``` > virt: > hw: > disk: !mux > virtio_blk: > disk_type: virtio_blk > virtio_scsi: > disk_type: virtio_scsi > ``` > > Now the user develops `virtio_scsi_next` and he wants to compare them. > Today he simply merges this config with the above: > > ``` > virt: > hw: > disk: !mux > virtio_scsi_debug: > disk_type: virtio_scsi > enable_next: True > ``` > and avocado produces 3 variants, where `params.get("disk_type", > "/virt/hw/disk/*")` reports the 3 defined variants. If we try to do the > same with `*/virt/hw/disk` we have to modify the first file: > > ``` > !mux > virtio_blk: > virt: > hw: > disk: > disk_type: virtio_blk > virtio_scsi: > virt: > hw: > disk: > disk_type: virtio_scsi > ``` > > One would want to prepend yet another node in front of it, because we > don't want to vary over disk types only, but also over other items (like > cpus, ...). The problem is, that the first category has to again be > unique to the whole multiplex tree in order to not clash with the other > items. And that is what the tree path was actually introduced, to get > rid of this global-namespace. > > Right now the only solution I see is to change the way `!mux` works. > Currently it multiplexes all the children, but (not sure if easily done) > it should only define the children, which mix together. Therefor (back > to the original example) one would be able to say: > > ``` > plugins: > virt: > qemu: > enabled: !newmux > kvm: on > disabled: !newmux > kvm: off > paths: > qemu_dst_bin: None > qemu_img_bin: None > qemu_bin: None > migrate: > timeout: 60.0 > ``` > > which would produce: > > ``` > ┗━━ plugins > ┗━━ virt > ┣━━ qemu > ┃ ╠══ enabled > ┃ ║ → kvm: on > ┃ ╠══ disabled > ┃ ┃ → kvm: off > ┃ ┣━━ paths > ┃ ┃ → qemu_dst_bin: None > ┃ ┃ → qemu_img_bin: None > ┃ ┃ → qemu_bin: None > ┃ ┗━━ migrate > ┃ → timeout: 60.0 > ``` > > and in terms of variants: > > ``` Even though this is an example, and we're worried about core concepts, I fail to see the point of the "/paths" and "/migrate" nodes here. Both "enabled" and "disabled" nodes, actually mean the user indent different multiplexed variants, while "/paths" and "/migrate" are "bins" for other values. It looks like your proposal for a new type of "!mux" tag/behavior is partially due to this mixed used of nodes (to be multiplexed and to serve as "bins" for misc values). > Variant 1: /plugins/virt/qemu/enabled, /plugins/virt/paths, > /plugins/virt/migrate > Variant 2: /plugins/virt/qemu/disabled, /plugins/virt/paths, > /plugins/virt/migrate > ``` > > I'm looking forward to your suggestions and I hope I'm wrong and that > the multiplexer (at least the full-spec) can handle this nicely. > > Kind regards, > Lukáš > -- Cleber Rosa [ Sr Software Engineer - Virtualization Team - Red Hat ] [ Avocado Test Framework - avocado-framework.github.io ]
signature.asc
Description: OpenPGP digital signature
