[Qemu-devel] Re: [RFC][PATCH v6 07/23] virtagent: base server definitions

[Jes Sorensen]

On 01/21/11 18:55, Michael Roth wrote:
> On 01/21/2011 10:38 AM, Jes Sorensen wrote:
>>> +#include<xmlrpc-c/base.h>
>>> +#include<xmlrpc-c/server.h>
>>> +
>>> +#define GUEST_AGENT_SERVICE_ID "virtagent"
>>> +#define GUEST_AGENT_PATH "/tmp/virtagent-guest.sock"
>>> +#define HOST_AGENT_SERVICE_ID "virtagent-host"
>>> +#define HOST_AGENT_PATH "/tmp/virtagent-host.sock"
>>> +#define VA_GETFILE_MAX 1<<  30
>>> +#define VA_FILEBUF_LEN 16384
>>> +#define VA_DMESG_LEN 16384
>>
>> I really don't like these hard coded constants - you you have a command
>> line interface allowing for the change of the sockets and file names?
>> Otherwise you'll hit problems on the host side with concurrent runs of
>> qemu.
> 
> Yup, that's one of the TODOs. In terms of configuration we can add
> parameters to the chardev to override these, but the goal here is sane
> defaults to avoid unnecessarily complicated invocations.

As a sane default, using <name>.pid or something along those lines is
better. It is very common to run more than one qemu instance at a time.

>> I really would like to see the dmesg stuff removed too for now as we
>> discussed earlier.
> 
> I think as a development/support tool it has a recently strong use case,
> even given it's limitations (which are not so bad....we retrieve up to a
> max of 16KB, possibly less depending on guest configuration, so it's not
> entirely predictable, but it's not dangerous. It's platform-specific,
> but that's handled by capabilities negotiation).

There is plenty of good ways to do the same thing, copy file to host,
then view is just as easy and can be scripted, without the security
implications of having it inline.

> I just don't really see the downside to keeping it in.

It's obviously contentious, and it is not core functionality. In order
to get the patches adapted upstream it would easy the process to remove
it and keep it as a separate patch.

Cheers,
Jes