After my first round of patches got merged [1], some new problems
arose. They are not as serious (except maybe for 7/9) but they are still
problematic for sure. Mostly in scenarios where a mgmt application uses
the embedded driver and has multiple forks/instances sharing essentially
the same config (e.g. it writes the same qemu.conf into different
roots). Thing is, we may generate conflicting paths across these
instances.

What I've done here is, I looked at what paths can be set from qemu.conf
and on their usage. Then I've grepped for virDomainDefGetShortName() and
looked at its usages, because that has proven to be common denominator
in patches 7-9.

Some usages of the function are safe though. When the usage involves a
path that is derived from the root and can't be overridden in qemu.conf
it is safe. For instance, qemuDBusGetAddress() calls
virDomainDefGetShortName() to construct a path to a UNIX socket. But the
path has cfg->dbusStateDir prefix which is derived from cfg->stateDir
which in turn is derived from the root. At the same time, neither of
these paths can be overridden in qemu.conf. Therefore, no conflicts can
occur.

Michal Prívozník (9):
  tests: Fix virQEMUDriverConfigNew() calling with respect to @root
  conf: Move virDomainGenerateMachineName to hypervisor/
  virDomainDriverGenerateMachineName: Factor out embed path hashing
  qemu: Introduce virQEMUDriverGetEmbedRoot
  qemuDomainGetMachineName: Access embeddedRoot from driver rather than
    cfg
  Revert "qemu_conf: Track embed root dir"
  qemu: Make hugepages path generation embed driver aware
  qemu: Make memory path generation embed driver aware
  qemu: Make auto dump path generation embed driver aware

 src/conf/domain_conf.c         | 72 ----------------------------
 src/conf/domain_conf.h         |  7 ---
 src/hypervisor/domain_driver.c | 88 ++++++++++++++++++++++++++++++++++
 src/hypervisor/domain_driver.h | 11 +++++
 src/libvirt_private.syms       |  3 +-
 src/lxc/lxc_domain.c           |  3 +-
 src/qemu/qemu_command.c        | 19 ++++----
 src/qemu/qemu_conf.c           | 64 ++++++++++++++++++-------
 src/qemu/qemu_conf.h           | 23 ++++-----
 src/qemu/qemu_domain.c         |  9 ++--
 src/qemu/qemu_driver.c         | 10 +++-
 src/qemu/qemu_process.c        |  7 ++-
 tests/domaincapstest.c         |  2 +-
 tests/testutilsqemu.c          |  2 +-
 tests/virsystemdtest.c         |  5 +-
 15 files changed, 191 insertions(+), 134 deletions(-)

-- 
2.24.1

On 3/25/20 7:18 AM, Michal Privoznik wrote:
> After my first round of patches got merged [1], some new problems
> arose. They are not as serious (except maybe for 7/9) but they are still
> problematic for sure. Mostly in scenarios where a mgmt application uses
> the embedded driver and has multiple forks/instances sharing essentially
> the same config (e.g. it writes the same qemu.conf into different
> roots). Thing is, we may generate conflicting paths across these
> instances.
> 
> What I've done here is, I looked at what paths can be set from qemu.conf
> and on their usage. Then I've grepped for virDomainDefGetShortName() and
> looked at its usages, because that has proven to be common denominator
> in patches 7-9.
> 
> Some usages of the function are safe though. When the usage involves a
> path that is derived from the root and can't be overridden in qemu.conf
> it is safe. For instance, qemuDBusGetAddress() calls
> virDomainDefGetShortName() to construct a path to a UNIX socket. But the
> path has cfg->dbusStateDir prefix which is derived from cfg->stateDir
> which in turn is derived from the root. At the same time, neither of
> these paths can be overridden in qemu.conf. Therefore, no conflicts can
> occur.
> 
> Michal Prívozník (9):
>    tests: Fix virQEMUDriverConfigNew() calling with respect to @root
>    conf: Move virDomainGenerateMachineName to hypervisor/
>    virDomainDriverGenerateMachineName: Factor out embed path hashing
>    qemu: Introduce virQEMUDriverGetEmbedRoot
>    qemuDomainGetMachineName: Access embeddedRoot from driver rather than
>      cfg
>    Revert "qemu_conf: Track embed root dir"
>    qemu: Make hugepages path generation embed driver aware
>    qemu: Make memory path generation embed driver aware
>    qemu: Make auto dump path generation embed driver aware
> 

LGTM


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>