5:32 a.m.
Currently, we share the idea of old & new def with domains. Users can
*-edit an object (domain, pool) which spawns a new internal
representation for them. This is referenced via
{domainObj,poolObj}->newDef [compared to ->def]. However, for pool we
were never overwriting def with newDef. This must be done on
pool-destroy (like we do analogically in domain detroy).
---
src/storage/storage_driver.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 88f3809..3b95c70 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -794,6 +794,10 @@ storagePoolDestroy(virStoragePoolPtr obj) {
if (pool->configFile == NULL) {
virStoragePoolObjRemove(&driver->pools, pool);
pool = NULL;
+ } else if (pool->newDef) {
+ virStoragePoolDefFree(pool->def);
+ pool->def = pool->newDef;
+ pool->newDef = NULL;
}
ret = 0;
@@ -804,7 +808,6 @@ cleanup:
return ret;
}
-
static int
storagePoolDelete(virStoragePoolPtr obj,
unsigned int flags) {
--
1.7.8.5
5:32 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Introduce --inactive for pool-dumpxml
Storage is one of the last domains in libvirt where we don't fully
utilize inactive and live XML. Okay, it might be because we don't
have support for that. So implement such support. However, we need
to fallback when talking to old daemon which doesn't support this
new flag.
---
src/storage/storage_driver.c | 10 ++++++++--
tools/virsh.c | 24 ++++++++++++++++++++++--
tools/virsh.pod | 4 +++-
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 3b95c70..0b65369 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -960,9 +960,10 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj,
{
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool;
+ virStoragePoolDefPtr def;
char *ret = NULL;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_DOMAIN_XML_INACTIVE, NULL);
storageDriverLock(driver);
pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
@@ -974,7 +975,12 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj,
goto cleanup;
}
- ret = virStoragePoolDefFormat(pool->def);
+ if ((flags & VIR_DOMAIN_XML_INACTIVE) && pool->newDef)
+ def = pool->newDef;
+ else
+ def = pool->def;
+
+ ret = virStoragePoolDefFormat(def);
cleanup:
if (pool)
diff --git a/tools/virsh.c b/tools/virsh.c
index 0354822..65774c1 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -10803,6 +10803,7 @@ static const vshCmdInfo info_pool_dumpxml[] = {
static const vshCmdOptDef opts_pool_dumpxml[] = {
{"pool", VSH_OT_DATA, VSH_OFLAG_REQ, N_("pool name or uuid")},
+ {"inactive", VSH_OT_BOOL, 0, N_("show inactive defined XML")},
{NULL, 0, 0, NULL}
};
@@ -10811,15 +10812,20 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd)
{
virStoragePoolPtr pool;
bool ret = true;
+ bool inactive = vshCommandOptBool(cmd, "inactive");
+ unsigned int flags = 0;
char *dump;
+ if (inactive)
+ flags |= VIR_DOMAIN_XML_INACTIVE;
+
if (!vshConnectionUsability(ctl, ctl->conn))
return false;
if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL)))
return false;
- dump = virStoragePoolGetXMLDesc(pool, 0);
+ dump = virStoragePoolGetXMLDesc(pool, flags);
if (dump != NULL) {
vshPrint(ctl, "%s", dump);
VIR_FREE(dump);
@@ -16124,7 +16130,8 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd)
bool ret = false;
virStoragePoolPtr pool = NULL;
virStoragePoolPtr pool_edited = NULL;
- unsigned int flags = 0;
+ unsigned int flags = VIR_DOMAIN_XML_INACTIVE;
+ char *tmp_desc = NULL;
if (!vshConnectionUsability(ctl, ctl->conn))
goto cleanup;
@@ -16133,6 +16140,19 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd)
if (pool == NULL)
goto cleanup;
+ /* Some old daemons don't support _INACTIVE flag */
+ if (!(tmp_desc = virStoragePoolGetXMLDesc(pool, flags))) {
+ if (last_error->code == VIR_ERR_INVALID_ARG) {
+ flags &= ~VIR_DOMAIN_XML_INACTIVE;
+ virFreeError(last_error);
+ last_error = NULL;
+ } else {
+ goto cleanup;
+ }
+ } else {
+ VIR_FREE(tmp_desc);
+ }
+
#define EDIT_GET_XML virStoragePoolGetXMLDesc(pool, flags)
#define EDIT_NOT_CHANGED \
vshPrint(ctl, _("Pool %s XML configuration not changed.\n"), \
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f83a29d..191c9f2 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2112,9 +2112,11 @@ Destroy the resources used by a given I<pool> object. This
operation
is non-recoverable. The I<pool> object will still exist after this
command, ready for the creation of new storage volumes.
-=item B<pool-dumpxml> I<pool-or-uuid>
+=item B<pool-dumpxml> [I<--inactive>] I<pool-or-uuid>
Returns the XML information about the I<pool> object.
+I<--inactive> tells virsh to dump pool configuration that will be used
+on next start of the pool as opposed to the current pool configuration.
=item B<pool-edit> I<pool-or-uuid>
--
1.7.8.5
5:38 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Introduce --inactive for pool-dumpxml
On Mon, Jun 25, 2012 at 12:32:48PM +0200, Michal Privoznik wrote:
Storage is one of the last domains in libvirt where we don't
fully
utilize inactive and live XML. Okay, it might be because we don't
have support for that. So implement such support. However, we need
to fallback when talking to old daemon which doesn't support this
new flag.
---
src/storage/storage_driver.c | 10 ++++++++--
tools/virsh.c | 24 ++++++++++++++++++++++--
tools/virsh.pod | 4 +++-
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 3b95c70..0b65369 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -960,9 +960,10 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj,
{
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool;
+ virStoragePoolDefPtr def;
char *ret = NULL;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_DOMAIN_XML_INACTIVE, NULL);
We can't just use the VIR_DOMAIN_XML_INACTIVE flag - we
must have a VIR_STORAGE_XML_INACTIVE flag here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:43 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Introduce --inactive for pool-dumpxml
On 25.06.2012 12:38, Daniel P. Berrange wrote:
On Mon, Jun 25, 2012 at 12:32:48PM +0200, Michal Privoznik wrote:
> Storage is one of the last domains in libvirt where we don't fully
> utilize inactive and live XML. Okay, it might be because we don't
> have support for that. So implement such support. However, we need
> to fallback when talking to old daemon which doesn't support this
> new flag.
> ---
> src/storage/storage_driver.c | 10 ++++++++--
> tools/virsh.c | 24 ++++++++++++++++++++++--
> tools/virsh.pod | 4 +++-
> 3 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 3b95c70..0b65369 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -960,9 +960,10 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj,
> {
> virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
> virStoragePoolObjPtr pool;
> + virStoragePoolDefPtr def;
> char *ret = NULL;
>
> - virCheckFlags(0, NULL);
> + virCheckFlags(VIR_DOMAIN_XML_INACTIVE, NULL);
We can't just use the VIR_DOMAIN_XML_INACTIVE flag - we
must have a VIR_STORAGE_XML_INACTIVE flag here.
Daniel
While I would fundamentally agree, unfortunately we've already
documented that flags passed to virStoragePoolGetXMLDesc() are typeof
virDomainXMLFlags;
On the other hand, there currently are no flags yet, so nobody uses this
nowadays. So let me resend with VIR_STORAGE_XML_INACTIVE.
Michal
6:02 a.m.
New subject: [libvirt] [PATCH 2/2] storage: Introduce --inactive for pool-dumpxml
On Mon, Jun 25, 2012 at 12:43:16PM +0200, Michal Privoznik wrote:
On 25.06.2012 12:38, Daniel P. Berrange wrote:
> On Mon, Jun 25, 2012 at 12:32:48PM +0200, Michal Privoznik wrote:
>> Storage is one of the last domains in libvirt where we don't fully
>> utilize inactive and live XML. Okay, it might be because we don't
>> have support for that. So implement such support. However, we need
>> to fallback when talking to old daemon which doesn't support this
>> new flag.
>> ---
>> src/storage/storage_driver.c | 10 ++++++++--
>> tools/virsh.c | 24 ++++++++++++++++++++++--
>> tools/virsh.pod | 4 +++-
>> 3 files changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index 3b95c70..0b65369 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -960,9 +960,10 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj,
>> {
>> virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
>> virStoragePoolObjPtr pool;
>> + virStoragePoolDefPtr def;
>> char *ret = NULL;
>>
>> - virCheckFlags(0, NULL);
>> + virCheckFlags(VIR_DOMAIN_XML_INACTIVE, NULL);
>
> We can't just use the VIR_DOMAIN_XML_INACTIVE flag - we
> must have a VIR_STORAGE_XML_INACTIVE flag here.
>
> Daniel
>
While I would fundamentally agree, unfortunately we've already
documented that flags passed to virStoragePoolGetXMLDesc() are typeof
virDomainXMLFlags;
On the other hand, there currently are no flags yet, so nobody uses this
nowadays. So let me resend with VIR_STORAGE_XML_INACTIVE.
Agreed, given that we have never supported any flags, I don't have a
problem fixing the docs. It was almost certainly a cut+paste mistake
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:36 a.m.
On Mon, Jun 25, 2012 at 12:32:47PM +0200, Michal Privoznik wrote:
Currently, we share the idea of old & new def with domains. Users
can
*-edit an object (domain, pool) which spawns a new internal
representation for them. This is referenced via
{domainObj,poolObj}->newDef [compared to ->def]. However, for pool we
were never overwriting def with newDef. This must be done on
pool-destroy (like we do analogically in domain detroy).
---
src/storage/storage_driver.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 88f3809..3b95c70 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -794,6 +794,10 @@ storagePoolDestroy(virStoragePoolPtr obj) {
if (pool->configFile == NULL) {
virStoragePoolObjRemove(&driver->pools, pool);
pool = NULL;
+ } else if (pool->newDef) {
+ virStoragePoolDefFree(pool->def);
+ pool->def = pool->newDef;
+ pool->newDef = NULL;
}
ret = 0;
@@ -804,7 +808,6 @@ cleanup:
return ret;
}
-
static int
storagePoolDelete(virStoragePoolPtr obj,
unsigned int flags) {
ACK,
while you're looking at this, pool-dumpxml and net-dumpxml are both
missing support for the --inactive flag, to view the inactive XML
while running
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:13 a.m.
On 25.06.2012 12:36, Daniel P. Berrange wrote:
On Mon, Jun 25, 2012 at 12:32:47PM +0200, Michal Privoznik wrote:
> Currently, we share the idea of old & new def with domains. Users can
> *-edit an object (domain, pool) which spawns a new internal
> representation for them. This is referenced via
> {domainObj,poolObj}->newDef [compared to ->def]. However, for pool we
> were never overwriting def with newDef. This must be done on
> pool-destroy (like we do analogically in domain detroy).
> ---
> src/storage/storage_driver.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 88f3809..3b95c70 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -794,6 +794,10 @@ storagePoolDestroy(virStoragePoolPtr obj) {
> if (pool->configFile == NULL) {
> virStoragePoolObjRemove(&driver->pools, pool);
> pool = NULL;
> + } else if (pool->newDef) {
> + virStoragePoolDefFree(pool->def);
> + pool->def = pool->newDef;
> + pool->newDef = NULL;
> }
> ret = 0;
>
> @@ -804,7 +808,6 @@ cleanup:
> return ret;
> }
>
> -
> static int
> storagePoolDelete(virStoragePoolPtr obj,
> unsigned int flags) {
ACK,
while you're looking at this, pool-dumpxml and net-dumpxml are both
missing support for the --inactive flag, to view the inactive XML
while running
Daniel
Thanks, pushed.
Actually, pool-dumpxml is being fixed in 2/2 (which we agreed I'll send
v2) and net-dumpxml was enhanced just recently - since
52d064f42dbc857f4096dc60c0335395ffac73aa. If I didn't look into this I
wouldn't notice neither.
Michal
9:29 a.m.
On Mon, Jun 25, 2012 at 01:13:48PM +0200, Michal Privoznik wrote:
On 25.06.2012 12:36, Daniel P. Berrange wrote:
> On Mon, Jun 25, 2012 at 12:32:47PM +0200, Michal Privoznik wrote:
>> Currently, we share the idea of old & new def with domains. Users can
>> *-edit an object (domain, pool) which spawns a new internal
>> representation for them. This is referenced via
>> {domainObj,poolObj}->newDef [compared to ->def]. However, for pool we
>> were never overwriting def with newDef. This must be done on
>> pool-destroy (like we do analogically in domain detroy).
>> ---
>> src/storage/storage_driver.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>> index 88f3809..3b95c70 100644
>> --- a/src/storage/storage_driver.c
>> +++ b/src/storage/storage_driver.c
>> @@ -794,6 +794,10 @@ storagePoolDestroy(virStoragePoolPtr obj) {
>> if (pool->configFile == NULL) {
>> virStoragePoolObjRemove(&driver->pools, pool);
>> pool = NULL;
>> + } else if (pool->newDef) {
>> + virStoragePoolDefFree(pool->def);
>> + pool->def = pool->newDef;
>> + pool->newDef = NULL;
>> }
>> ret = 0;
>>
>> @@ -804,7 +808,6 @@ cleanup:
>> return ret;
>> }
>>
>> -
>> static int
>> storagePoolDelete(virStoragePoolPtr obj,
>> unsigned int flags) {
>
> ACK,
>
> while you're looking at this, pool-dumpxml and net-dumpxml are both
> missing support for the --inactive flag, to view the inactive XML
> while running
>
> Daniel
>
Thanks, pushed.
Belated functional ACK, that makes things work for me; thanks!
Dave
Actually, pool-dumpxml is being fixed in 2/2 (which we agreed
I'll send
v2) and net-dumpxml was enhanced just recently - since
52d064f42dbc857f4096dc60c0335395ffac73aa. If I didn't look into this I
wouldn't notice neither.
Michal
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4536
days inactive
4536
days old
7 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Dave Allan -
Michal Privoznik