[libvirt] [PATCH RFC 1/4] qemu_agent: move agent into util

Joao Martins joao.m.martins at oracle.com
Fri Mar 3 11:36:25 UTC 2017



On 03/02/2017 05:58 PM, Jim Fehlig wrote:
> Sorry for the review delay.
> 
> On 02/08/2017 09:44 AM, Joao Martins wrote:
>> As it could be shared with libxl which now allows channels to
>> be created. Also changed filename to match others in the same
>> directory namely to virqemuagent.{h,c}
>>
>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>> ---
>>  po/POTFILES.in               |    2 +-
>>  src/Makefile.am              |    2 +-
>>  src/libvirt_private.syms     |   21 +
>>  src/qemu/qemu_agent.c        | 2248 ------------------------------------------
>>  src/qemu/qemu_agent.h        |  123 ---
>>  src/qemu/qemu_domain.h       |    2 +-
>>  src/qemu/qemu_driver.c       |    2 +-
>>  src/util/virqemuagent.c      | 2248 ++++++++++++++++++++++++++++++++++++++++++
>>  src/util/virqemuagent.h      |  123 +++
>>  tests/qemuagenttest.c        |    2 +-
>>  tests/qemumonitortestutils.c |    2 +-
>>  tests/qemumonitortestutils.h |    2 +-
>>  12 files changed, 2399 insertions(+), 2378 deletions(-)
>>  delete mode 100644 src/qemu/qemu_agent.c
>>  delete mode 100644 src/qemu/qemu_agent.h
>>  create mode 100644 src/util/virqemuagent.c
>>  create mode 100644 src/util/virqemuagent.h
> 
> I hope others will opine on this change. It seems reasonable to me and I'm 
> surprised the qemu driver only needed tiny changes to accommodate moving all 
> this code.

Yeap, I too was surprised on how the changes were non disruptive wrt to qemu
driver :)

> 
> [...]
>> diff --git a/src/util/virqemuagent.h b/src/util/virqemuagent.h
>> new file mode 100644
>> index 0000000..2e81020
>> --- /dev/null
>> +++ b/src/util/virqemuagent.h
>> @@ -0,0 +1,123 @@
>> +/*
>> + * virqemuagent.h: interaction with QEMU guest agent
>> + *
>> + * Copyright (C) 2006-2012 Red Hat, Inc.
>> + * Copyright (C) 2006 Daniel P. Berrange
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Daniel P. Berrange <berrange at redhat.com>
>> + */
>> +
>> +
>> +#ifndef __QEMU_AGENT_H__
>> +# define __QEMU_AGENT_H__
>> +
>> +# include "internal.h"
>> +# include "domain_conf.h"
> 
> ISTR a recent discussion on the list frowning on using code from src/conf in 
> src/util, although virthostdev.h also includes domain_conf.h.
> 
> BTW, compilation fails here
> 
> In file included from util/virqemuagent.c:35:0:
> util/virqemuagent.h:29:26: fatal error: domain_conf.h: No such file or directory
>   # include "domain_conf.h"
> 
Hmm, will have to fix it for v1. This series were applied on top of commit
2841e67 ("qemu: propagate bridge MTU into qemu "host_mtu" option") and I didn't
observe compilation  issues (neither make {syntax-check,check} IIRC).

>> +
>> +typedef struct _qemuAgent qemuAgent;
>> +typedef qemuAgent *qemuAgentPtr;
>> +
>> +typedef struct _qemuAgentCallbacks qemuAgentCallbacks;
>> +typedef qemuAgentCallbacks *qemuAgentCallbacksPtr;
>> +struct _qemuAgentCallbacks {
>> +    void (*destroy)(qemuAgentPtr mon,
>> +                    virDomainObjPtr vm);
>> +    void (*eofNotify)(qemuAgentPtr mon,
>> +                      virDomainObjPtr vm);
>> +    void (*errorNotify)(qemuAgentPtr mon,
>> +                        virDomainObjPtr vm);
>> +};
>> +
>> +
>> +qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm,
>> +                           const virDomainChrSourceDef *config,
>> +                           qemuAgentCallbacksPtr cb);
>> +
>> +void qemuAgentClose(qemuAgentPtr mon);
> 
> Other files in src/util prefix structs and functions with "vir". I'm not sure 
> how picky folks are about that. If the consensus is towards the "vir" prefix, 
> perhaps it would be easier done with a follow-up after the move?
Yeah I noticed the pattern. Though given the big move of code into util making
as a follow-up patch would easy review perhaps.

> 
> Thanks a lot for working on this feature! It would be cool to get it in the 
> 3.2.0 release.
> 
> Regards,
> Jim
> 




More information about the libvir-list mailing list