[libvirt] [PATCH 01/12] qemu: Add a hash table for the shared disks
Osier Yang
jyang at redhat.com
Thu Dec 13 11:14:41 UTC 2012
On 2012年12月13日 19:05, Jiri Denemark wrote:
> On Tue, Dec 11, 2012 at 21:37:18 +0800, Osier Yang wrote:
>> This introduces a hash table for qemu driver, to store the shared
>> disk's info as (@disk_path, {@ref_count, @orig_cdbfilter}). @ref_count
>> is the number of domains which shares the disk. @orig_cdbfilter is
>> the original cdbfilter setting of the shared disk, it will be used
>> to restore the the disk's cdbfilter setting to original value by
>> later patches.
>>
>> * src/qemu/qemu_conf.h: (Add member 'sharedDisks' of type
>> virHashTablePtr; New struct qemuSharedDiskEntry;
>> Declare helpers qemuAddSharedDisk,
>> qemuRemoveSharedDisk)
>> * src/qemu/qemu_conf.c (Implement the two helpers)
>> * src/qemu/qemu_process.c (Update 'sharedDisks' when domain
>> starting and shutdown)
>> * src/qemu/qemu_driver.c (Update 'sharedDisks' when attaching
>> or detaching disk).
>>
>> 0 is passed for orig_cdbfilter temporarily, later patches will update
>> it.
>> ---
>> src/qemu/qemu_conf.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_conf.h | 18 +++++++++++++++++
>> src/qemu/qemu_driver.c | 17 ++++++++++++++++
>> src/qemu/qemu_process.c | 17 +++++++++++++++-
>> 4 files changed, 99 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 8d380a1..2b21186 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -556,3 +556,51 @@ qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver,
>>
>> virHashForEach(driver->closeCallbacks, qemuDriverCloseCallbackRun,&data);
>> }
>> +
>> +/* Increase ref count if the entry already exists, otherwise
>> + * add a new entry.
>> + */
>> +int
>> +qemuAddSharedDisk(virHashTablePtr sharedDisks,
>> + const char *disk_path,
>> + int orig_cdbfilter)
>> +{
>> + qemuSharedDiskEntryPtr entry = NULL;
>> +
>> + if ((entry = virHashLookup(sharedDisks, disk_path))) {
>> + entry->ref++;
>> + } else {
>> + if (VIR_ALLOC(entry)< 0)
>> + return -1;
>> +
>> + entry->ref = 1;
>> + entry->orig_cdbfilter = orig_cdbfilter;
>> +
>> + if (virHashAddEntry(sharedDisks, disk_path, entry))
>> + return -1;
>> + }
>
> In addition to Daniel's comments, I think this hash table should contain
> device number rather than disk path since /dev is full of symlinks. One
> domain may be configured to use /dev/sdg while another one would use
> /dev/disk/by-id/..., another one /dev/disk/by-uuid/... and they can all
> lead to the same block device. Thus you need to canonicalize disk_path
> first, e.g., into major:minor and store that in the hash table.
>
Hum, agreed, this is really good catch which saves us from the
potential bug. Thanks.
Regards,
Osier
More information about the libvir-list
mailing list