Re: [libvirt] [PATCH] qemu: Don't bother user with libvirt-internal paths

On Thu, Jan 07, 2016 at 09:40:29AM +0100, Pavel Hrdina wrote: >On Wed, Jan 06, 2016 at 05:50:57PM +0100, Martin Kletzander wrote: >> If user defines a virtio channel with UNIX socket backend and doesn't >> care about the path for the socket (e.g. qemu-agent channel), we still >> generate it into the persistent XML. Moreover when then user renames >> the domain, due to its persistent socket path saved into the per-domain >> directory, it will not start. So let's forget about old generated paths >> and also stop putting them into the persistent definition. >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1278068 >> >> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >> --- >> src/qemu/qemu_command.c | 12 ++++++++++++ >> src/qemu/qemu_domain.c | 21 +++++++++++---------- >> .../qemuxml2argv-channel-virtio-unix.args | 5 ++++- >> .../qemuxml2argv-channel-virtio-unix.xml | 4 ++++ >> 4 files changed, 31 insertions(+), 11 deletions(-) >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 66ca11152ad8..c5127cfa04f9 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -10510,6 +10510,18 @@ qemuBuildCommandLine(virConnectPtr conn, >> goto error; >> } >> >> + if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && >> + !channel->source.data.nix.path) { >> + if (virAsprintf(&channel->source.data.nix.path, >> + "%s/domain-%s/%s", >> + cfg->channelTargetDir, def->name, >> + channel->target.name ? channel->target.name >> + : "unknown.sock") < 0) >> + goto error; >> + >> + channel->source.data.nix.listen = true; >> + } >> + > >I don't like this. The qemuBuildCommandLine function should only create a >command line, not modify the domain definition. I know, there are other places, >that do the same, but let's try to avoid it. > But it's the only function that gets involved from the tests. We should have different places for generating stuff and formatting command line, I know, it looks awful. But for now it fixes the issue and I wasn't sure whether Jiri wasn't messing with any other qemuProcess{Init,Launch,Start,Whatever} renames and rebuilds. Is there any place now that we could use without refactoring the whole thing?

>> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && >> channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { >> /* spicevmc was originally introduced via a -device >> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > >[...] > >Otherwise it looks good. > >Pavel