[Libvir] Fix handling of config files with duplicate named guests
The code for managing config files in /etc/xen assumed that the filename of the config matched the name of the guest defined inside. One might thing this a reasonable assumption, but in the wild I've had reports from users whom have various config files for the same guest, but with different settings. This caused some really wierd behaviour in the current code base. virt-manager display would alternate displaying each config upon refresh! Now, our API requires that there is only one config per name, so we can't expose this to the UI. The attached patch, thus simply detects when we have more than one config with same named guest, and hides the duplicates. The way it does this is to change the way we cache configs. Previously we had a single hash table mapping relative filenames to virConfPtr objects, with the implicit assumption that relative filename == guest name. With the attached patch we now maintain two hash tables. The first maps fully qualified pathnames to virConfPtr objects, the second maps guest names to the first filename found for that name. Thus when querying details for a guest, we first resolve the guest name to its filename, and then lookup the virConfPtr object associated with this filename. This isn't perfect, but its a hell of alot better than current code, so I want to commit it as is and figure out more improvement later. There is no ABI issue here, so we can iterate over impl at will. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Dec 19, 2006 at 06:48:16PM +0000, Daniel P. Berrange wrote:
The code for managing config files in /etc/xen assumed that the filename of the config matched the name of the guest defined inside. One might thing this a reasonable assumption, but in the wild I've had reports from users whom have various config files for the same guest, but with different settings. This caused some really wierd behaviour in the current code base. virt-manager display would alternate displaying each config upon refresh!
Now, our API requires that there is only one config per name, so we can't expose this to the UI. The attached patch, thus simply detects when we have more than one config with same named guest, and hides the duplicates.
The way it does this is to change the way we cache configs. Previously we had a single hash table mapping relative filenames to virConfPtr objects, with the implicit assumption that relative filename == guest name. With the attached patch we now maintain two hash tables. The first maps fully qualified pathnames to virConfPtr objects, the second maps guest names to the first filename found for that name. Thus when querying details for a guest, we first resolve the guest name to its filename, and then lookup the virConfPtr object associated with this filename.
This isn't perfect, but its a hell of alot better than current code, so I want to commit it as is and figure out more improvement later. There is no ABI issue here, so we can iterate over impl at will.
okay sounds fine to me...
+ value = malloc(sizeof(virConfValue)); + value->str = strdup(filename); + free(value); + return (-1);
This makes me think about an idea I had to swicth all allocations in libvirt to reuse xmlMalloc/xmlFree/xmlStrdup . The reasons are the following: - libxml2 is a mandatory module - libxml2 memory debug allow to track easilly memory allocations and check we never leak, this can be really useful - libxml2 memory routines can also be overiden by the library client to use a different allocator which in some circumstances can be really useful My prime motivation is debug but reuse of libxml2 facilities is not neglectible. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Tue, Dec 19, 2006 at 02:05:32PM -0500, Daniel Veillard wrote:
On Tue, Dec 19, 2006 at 06:48:16PM +0000, Daniel P. Berrange wrote:
The code for managing config files in /etc/xen assumed that the filename of the config matched the name of the guest defined inside. One might thing this a reasonable assumption, but in the wild I've had reports from users whom have various config files for the same guest, but with different settings. This caused some really wierd behaviour in the current code base. virt-manager display would alternate displaying each config upon refresh!
Now, our API requires that there is only one config per name, so we can't expose this to the UI. The attached patch, thus simply detects when we have more than one config with same named guest, and hides the duplicates.
The way it does this is to change the way we cache configs. Previously we had a single hash table mapping relative filenames to virConfPtr objects, with the implicit assumption that relative filename == guest name. With the attached patch we now maintain two hash tables. The first maps fully qualified pathnames to virConfPtr objects, the second maps guest names to the first filename found for that name. Thus when querying details for a guest, we first resolve the guest name to its filename, and then lookup the virConfPtr object associated with this filename.
This isn't perfect, but its a hell of alot better than current code, so I want to commit it as is and figure out more improvement later. There is no ABI issue here, so we can iterate over impl at will.
okay sounds fine to me...
Ok, will commit.
+ value = malloc(sizeof(virConfValue)); + value->str = strdup(filename); + free(value); + return (-1);
This makes me think about an idea I had to swicth all allocations in libvirt to reuse xmlMalloc/xmlFree/xmlStrdup . The reasons are the following: - libxml2 is a mandatory module - libxml2 memory debug allow to track easilly memory allocations and check we never leak, this can be really useful - libxml2 memory routines can also be overiden by the library client to use a different allocator which in some circumstances can be really useful
My prime motivation is debug but reuse of libxml2 facilities is not neglectible.
Sounds reasonble. I'm not familiar with libxml2 debug stuff though - does it give more info that we can get from valgrind ? Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Dec 19, 2006 at 07:26:26PM +0000, Daniel P. Berrange wrote:
On Tue, Dec 19, 2006 at 02:05:32PM -0500, Daniel Veillard wrote:
+ value = malloc(sizeof(virConfValue)); + value->str = strdup(filename); + free(value); + return (-1);
This makes me think about an idea I had to swicth all allocations in libvirt to reuse xmlMalloc/xmlFree/xmlStrdup . The reasons are the following: - libxml2 is a mandatory module - libxml2 memory debug allow to track easilly memory allocations and check we never leak, this can be really useful - libxml2 memory routines can also be overiden by the library client to use a different allocator which in some circumstances can be really useful
My prime motivation is debug but reuse of libxml2 facilities is not neglectible.
Sounds reasonble. I'm not familiar with libxml2 debug stuff though - does it give more info that we can get from valgrind ?
Frankly no, but there isn't the same cost penalty. Also one can track a given piece of memory, for example under gdb. Documentation at: http://xmlsoft.org/xmlmem.html Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Tue, Dec 19, 2006 at 04:35:00PM -0500, Daniel Veillard wrote:
On Tue, Dec 19, 2006 at 07:26:26PM +0000, Daniel P. Berrange wrote:
On Tue, Dec 19, 2006 at 02:05:32PM -0500, Daniel Veillard wrote:
+ value = malloc(sizeof(virConfValue)); + value->str = strdup(filename); + free(value); + return (-1);
This makes me think about an idea I had to swicth all allocations in libvirt to reuse xmlMalloc/xmlFree/xmlStrdup . The reasons are the following: - libxml2 is a mandatory module - libxml2 memory debug allow to track easilly memory allocations and check we never leak, this can be really useful - libxml2 memory routines can also be overiden by the library client to use a different allocator which in some circumstances can be really useful
My prime motivation is debug but reuse of libxml2 facilities is not neglectible.
Sounds reasonble. I'm not familiar with libxml2 debug stuff though - does it give more info that we can get from valgrind ?
Frankly no, but there isn't the same cost penalty. Also one can track a given piece of memory, for example under gdb. Documentation at: http://xmlsoft.org/xmlmem.html
Actually there is one very useful usecase - multi threading. Valgrind is not multi-threaded at all - it more or less just runs each thread in turn for a few instructions before switching to next thread & so on. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (2)
-
Daniel P. Berrange -
Daniel Veillard