[libvirt] [PATCH v3 3/8] test: Implement readonly snapshot APIs

[Cole Robinson]

On 09/26/2013 10:44 AM, Michal Privoznik wrote:
> On 25.09.2013 21:15, Cole Robinson wrote:
>> This is just stolen from qemu_driver.c with tweaks to fit the
>> test driver.
>> ---
>>  src/test/test_driver.c | 392 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 392 insertions(+)
>>
>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> index 7e60716..9a39087 100644
>> --- a/src/test/test_driver.c
>> +++ b/src/test/test_driver.c
>> @@ -45,6 +45,7 @@
>>  #include "interface_conf.h"
>>  #include "domain_conf.h"
>>  #include "domain_event.h"
>> +#include "snapshot_conf.h"
>>  #include "fdstream.h"
>>  #include "storage_conf.h"
>>  #include "node_device_conf.h"
>> @@ -423,6 +424,27 @@ static const unsigned long long defaultPoolAlloc = 0;
>>  static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr pool);
>>  static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info);
>>  
>> +static virDomainObjPtr
>> +testDomObjFromDomain(virDomainPtr domain)
>> +{
>> +    virDomainObjPtr vm;
>> +    testConnPtr driver = domain->conn->privateData;
>> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
>> +
>> +    testDriverLock(driver);
>> +    vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
>> +    if (!vm) {
>> +        virUUIDFormat(domain->uuid, uuidstr);
>> +        virReportError(VIR_ERR_NO_DOMAIN,
>> +                       _("no domain with matching uuid '%s' (%s)"),
>> +                       uuidstr, domain->name);
>> +        vm = NULL;
> 
> This line is NOP. But we have the very same line in qemu driver too. So
> please remove this and post patch that cleanups *DomObjFromDomain()
> functions (quick look into libxl and lxc drivers shows these functions
> can be simplified a bit).
> 

I dropped it from this patch, but the qemu driver isn't affected anymore since
that code was recently altered.

>> +    }
>> +
>> +    testDriverUnlock(driver);
>> +    return vm;
>> +}
> 
> Once we have this function a follow up patch would be nice - turn all these
> 
> testDriverLock()
> virDomainObjListFindByName()
> testDriverUnlock()
> 
> into testDomObjFromDomain().
> 

Yeah there's definitely a lot of opportunity for such cleanups in the test driver.

>> +
>>  static char *
>>  testDomainGenerateIfname(virDomainDefPtr domdef) {
>>      int maxif = 1024;
>> @@ -6098,6 +6120,362 @@ cleanup:
>>  }
>>  
>>  
>> +/*
>> + * Snapshot APIs
>> + */
>> +
> 
> Yup, these are true copy of qemu functions.
> 
>>  static virDriver testDriver = {
>>      .no = VIR_DRV_TEST,
>>      .name = "Test",
>> @@ -6174,6 +6552,20 @@ static virDriver testDriver = {
>>      .domainManagedSave = testDomainManagedSave, /* 1.1.3 */
>>      .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.3 */
>>      .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.3 */
>> +
>> +    .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.3 */
>> +    .domainSnapshotListNames = testDomainSnapshotListNames, /* 1.1.3 */
>> +    .domainListAllSnapshots = testDomainListAllSnapshots, /* 1.1.3 */
>> +    .domainSnapshotGetXMLDesc = testDomainSnapshotGetXMLDesc, /* 1.1.3 */
>> +    .domainSnapshotNumChildren = testDomainSnapshotNumChildren, /* 1.1.3 */
>> +    .domainSnapshotListChildrenNames = testDomainSnapshotListChildrenNames, /* 1.1.3 */
>> +    .domainSnapshotListAllChildren = testDomainSnapshotListAllChildren, /* 1.1.3 */
>> +    .domainSnapshotLookupByName = testDomainSnapshotLookupByName, /* 1.1.3 */
>> +    .domainHasCurrentSnapshot = testDomainHasCurrentSnapshot, /* 1.1.3 */
>> +    .domainSnapshotGetParent = testDomainSnapshotGetParent, /* 1.1.3 */
>> +    .domainSnapshotCurrent = testDomainSnapshotCurrent, /* 1.1.3 */
>> +    .domainSnapshotIsCurrent = testDomainSnapshotIsCurrent, /* 1.1.3 */
>> +    .domainSnapshotHasMetadata = testDomainSnapshotHasMetadata, /* 1.1.3 */
>>  };
>>  
>>  static virNetworkDriver testNetworkDriver = {
>>
> 
> Unfortunately, we are in the freeze, so you'll need to update these to
> 1.1.4 and push after the release.
> 
> ACK
> 

Pushed with those changes.

Thanks,
Cole