On 05/19/2016 04:50 PM, Cole Robinson wrote:
> On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
>> Both VNC and SPICE requires the same code to resolve address for listen
>> type network. Remove code duplication and create a new function that
>> will be used in qemuProcessSetupGraphics().
>>
>> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_command.c | 103 ++++++------------------------------------------
>> src/qemu/qemu_process.c | 47 +++++++++++++++++++++-
>> 2 files changed, 57 insertions(+), 93 deletions(-)
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index f4bf6c1..75c8e53 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -4388,6 +4388,32 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
>>
>>
>> static int
>> +qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten,
>> + const char *listenAddr)
>> +{
>> + int rc;
>> +
>> + if (!glisten->network) {
>> + if (VIR_STRDUP(glisten->address, listenAddr) < 0)
>> + return -1;
>> + return 0;
>> + }
>> +
>
> This is a logic change. Previously we accept this XML
>
> <graphics ...>
> <listen type='network'/>
> </graphics>
>
> and silently treat that as using vnc_listen/spice_listen. Now we stick that
> address in the XML like
>
> <graphics ...>
> <listen type='network' address='$vnc_listen'/>
> </graphics>
>
> Which at least is more explicit, but it is a logic change. Just shows that the
> address type='network' stuff needs more test coverage at least. I think at
> some point we should reject bare type='network' if it doesn't have a @network
> attribute
>
> If that change was intentional it should be an additive patch after this
> cleanup, with a test case
>
Hmm okay I see that it must be intentional, because the qemu_command code now
depends on glisten->address. So ACK to this as long as you add a todo item to
add some test cases for this stuff, and to figure out the bare <listen
type='network'/> weirdness
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list