[Avocado-devel] (potential) design issue in multiplexer

[Ademar Reis]

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

> 
> ```
>  ┗━━ 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
> ```
> 
> 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:
> 
> ```
>  ┣━━ 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:
> 
> ```
> 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áš
> 




-- 
Ademar Reis
Red Hat

^[:wq!