[libvirt] [PATCH 3/9] New files to glue domain and storage together

[Osier Yang]

On 2013年02月04日 22:30, Daniel P. Berrange wrote:
> On Thu, Jan 31, 2013 at 02:11:28AM +0800, Osier Yang wrote:
>> This creates src/conf/domain_storage.h and src/conf/storage_conf.c,
>> which defines a driver contains internal APIs to glue the domain
>> and storage. Currently there is only one API, translateDiskSourcePool,
>> which is to translate the specified pool/volume into the the real
>> underlying disk source. Later patch will implement API.
>> ---
>>   src/Makefile.am           |    3 ++-
>>   src/conf/domain_storage.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>>   src/conf/domain_storage.h |   37 +++++++++++++++++++++++++++++++++++++
>>   src/libvirt_private.syms  |    4 ++++
>>   4 files changed, 87 insertions(+), 1 deletions(-)
>>   create mode 100644 src/conf/domain_storage.c
>>   create mode 100644 src/conf/domain_storage.h
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 070a089..d664c48 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -203,7 +203,8 @@ DOMAIN_CONF_SOURCES =						\
>>   		conf/domain_conf.c conf/domain_conf.h		\
>>   		conf/domain_audit.c conf/domain_audit.h		\
>>   		conf/domain_nwfilter.c conf/domain_nwfilter.h	\
>> -		conf/snapshot_conf.c conf/snapshot_conf.h
>> +		conf/snapshot_conf.c conf/snapshot_conf.h	\
>> +		conf/domain_storage.c conf/domain_storage.h
>>
>>   DOMAIN_EVENT_SOURCES =						\
>>   		conf/domain_event.c conf/domain_event.h
>> diff --git a/src/conf/domain_storage.c b/src/conf/domain_storage.c
>> new file mode 100644
>> index 0000000..299e9fd
>> --- /dev/null
>> +++ b/src/conf/domain_storage.c
>> @@ -0,0 +1,44 @@
>> +/*
>> + * domain_storage.c: Internal APIs to glue domain and storage
>> + *
>> + * Copyright (C) 2013 Red Hat, Inc.
>> + *
>> + * 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: Osier Yang<jyang at redhat.com>
>> + */
>> +
>> +#include<config.h>
>> +
>> +#include "internal.h"
>> +
>> +#include "datatypes.h"
>> +#include "domain_conf.h"
>> +#include "domain_storage.h"
>> +
>> +static virDomainStorageDriverPtr domainStorageDriver;
>> +
>> +void
>> +virRegisterDomainStorageDriver(virDomainStorageDriverPtr driver) {
>> +    domainStorageDriver = driver;
>> +}
>> +
>> +int
>> +virDomainStorageTranslateDiskSourcePool(virConnectPtr conn,
>> +                                        virDomainDefPtr def) {
>> +    if (domainStorageDriver != NULL)
>> +        return domainStorageDriver->translateDiskSourcePool(conn, def);
>> +    return 0;
>> +}
>
>
> IMHO this abstraction is rather pointless. Just call virStoragePoolLookupbyUUID()
> at the time you need to generate the CLI args, just as we do with the existing
> <interface type='network'/>

I thought you agreed with using this method on RFC. :-)

I'm wondering how much advantage we will get from invoking public
API instead of doing the work directly inside the server side.

If you look futher more on the later patches, especially 9/9, which
shows we will not only need the pool/volume object, but also the
pool/volume defs (to get either the underlying target path or
the underlying volume desciption, this is different with the domain
network, which only needs the network obj), which can't get by the
public APIs. Of course, this is not the problem, as we can introduce
some internal helpers to get the pool/volume defs. But I'm not sure
how much benifit we will get from that way.

Osier