Dne 17.8.2016 v 16:33 Cleber Rosa napsal(a):


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?

yep, sorry, I had to modify the examples couple of times due to https://github.com/avocado-framework/avocado/pull/1409


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.

No, that's not what I had in mind. The original RFC suggested that users should usually supply relative path like `qemu/paths` and the multiplexer should search the whole tree for all occurrences. I saw some benefits, but a lot of clashes, therefor I suggested the list of `multiplex_paths`, which define slices of the tree and those relative paths are searched there. Currently when relative path is specified, it only looks into the `multiplex_paths` (by default `/run` only) and it omits the rest (the `/`).

The consequence is, that those `/plugins` can't be queried for using relative paths (unless one specifies `multiplex_path += "/"` or "/plugins").

The following example shows how that would work if avocado supported this feature and we tried to query for `plugins/virt/qemu`. Also note there is a typo in the example, see the changes below:


```
 ┣━━ 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
it can't find the `plugins/virt/qemu`

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
the same here, it should be `plugins/virt/qemu`

2. it looks into `/` and finds it in `/my/kvm/plugins/virt/qemu` and in
as well as here `plugins/virt/qemu`

`/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).

The problem I'm trying to describe is, that when users try to multiplex non-leaf nodes, the `!mux` makes turns the existing nodes/children into a different variants and there is no way to avoid it apart from removing the node and recreating it. Hopefully it's clearer when I respond to Ademar with direct examples.

Regards,
Lukáš

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áš



Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to