Dne 17.8.2016 v 13:30 Ademar Reis napsal(a):
Sure, I thought this suffices, but you're right that real examples are easier to fiddle with.On Wed, Aug 17, 2016 at 09:43:34AM +0200, Lukáš Doktor wrote:Hello guys,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):It's been a while since I thought about the multiplexer at this level and I'm having trouble understanding the issue... Could you please provide an example using the avocado command line and yaml files where we can see the issue by running it and comparing the results?
Thanks. - Ademar
Let's create a `basic.yaml` (avocado does not accept empty multiplex files as they don't provide mapping)
``` foo: bar ``` and run ``` avocado multiplex --system-wide -t -i empty.yaml ```
``` ┗━━ 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:
Let's call this one `qemu.yaml`
``` plugins: virt: qemu: !mux enabled: kvm: on disabled: qemu: off ``` The problem is, that it affects the whole `/plugins/virt/qemu` node and all it's children become multiplexed:
and execute ``` avocado multiplex --system-wide -t -i /:qemu.yaml ``` (the `/` is important, otherwise it gets injected into `/run`)
``` ┗━━ 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:
``` avocado multiplex --system-wide -t -i /my/kvm:qemu.yaml ```
To get this result you need to insert the `Something_else` value. Here I'm using `--mux-inject` but imagine it's defined in the `--system-wide` (as was proposed in the original pull request and as is with the other values here like `qemu_dst_bin`).``` ┣━━ 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):
```avocado multiplex --system-wide -t -i /my/kvm:qemu.yaml --mux-inject /plugins/virt/qemu/paths:kvm:Something_else
```
``` ┣━━ 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 ```
which produces following params: ``` $ avocado multiplex /my/kvm:qemu.yaml -c --system-wide Variants generated:Variant 1: /my/kvm/plugins/virt/qemu/enabled, /plugins/virt/qemu/paths, /plugins/virt/qemu/migrate, /plugins/virt/paths, /plugins/virt/guest, /plugins/virt/screendumps, /plugins/virt/videos
/my/kvm/plugins/virt/qemu/enabled:kvm => True
/plugins/virt/guest:disable_restore_image_test => False
/plugins/virt/guest:image_path => None
/plugins/virt/guest:password => None
/plugins/virt/guest:user => None
/plugins/virt/paths:qemu_io_bin => None
/plugins/virt/qemu/migrate:timeout => 60.0
/plugins/virt/qemu/paths:qemu_bin => None
/plugins/virt/qemu/paths:qemu_dst_bin => None
/plugins/virt/qemu/paths:qemu_img_bin => None
/plugins/virt/screendumps:enable => None
/plugins/virt/screendumps:interval => 0.5
/plugins/virt/videos:enable => None
/plugins/virt/videos:jpeg_quality => 95
Variant 2: /my/kvm/plugins/virt/qemu/disabled,
/plugins/virt/qemu/paths, /plugins/virt/qemu/migrate,
/plugins/virt/paths, /plugins/virt/guest, /plugins/virt/screendumps,
/plugins/virt/videos
/my/kvm/plugins/virt/qemu/disabled:kvm => False
/plugins/virt/guest:disable_restore_image_test => False
/plugins/virt/guest:image_path => None
/plugins/virt/guest:password => None
/plugins/virt/guest:user => None
/plugins/virt/paths:qemu_io_bin => None
/plugins/virt/qemu/migrate:timeout => 60.0
/plugins/virt/qemu/paths:qemu_bin => None
/plugins/virt/qemu/paths:qemu_dst_bin => None
/plugins/virt/qemu/paths:qemu_img_bin => None
/plugins/virt/screendumps:enable => None
/plugins/virt/screendumps:interval => 0.5
/plugins/virt/videos:enable => None
/plugins/virt/videos:jpeg_quality => 95
```
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:
This one is `disks.yaml`
``` 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:
This one `mydisks.yaml`
``` virt: hw: disk: !mux virtio_scsi_debug: disk_type: virtio_scsi enable_next: True ```
we can try it out by running `avocado multiplex disks.yaml mydisks.yaml -c`
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:
`relative_disks.yaml`
``` !mux virtio_blk: virt: hw: disk: disk_type: virtio_blk virtio_scsi: virt: hw: disk: disk_type: virtio_scsi ```
Could be tested: ``` avocado multiplex relative_disks.yaml ```But in reality (to avoid multiplexing the root, which contains other values like `/run` and `/plugins`) one would use:
``` avocado multiplex /my_disks:relative_disks.yaml ```You are free to extend this, but the `my_disks` is the global variable shared across all your disk types and you must put them all there.
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:
No example here as it's not supported.
``` 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: ``` 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áš
After thinking about this a bit more, one way to solve it is also to restrict `!mux` definition on exiting non-mux nodes with children. In such case one would not be able to multiplex the `kvm/nokvm` as defined here without the change to the main structure (moving the `kvm=on|off` into a leaf node). This solutions still suffers one issue and that is multiple files where first defines the node without children, then one adds custom node redefining it to !mux and last another mux file which assumes the node is non-mux adds other children nodes resulting in unexpected behavior. This and the fact that it requires changes to the code when clash is found makes me think that it's not a good idea (also valid simplification).
I hope it's clearer with the examples, Lukáš
signature.asc
Description: OpenPGP digital signature
