When using the --start option, the show_count should not be set to
max_id as the --start means we dont need those many inital cpu
stats. Hence, show_count should be adjusted accordingly.
Signed-off-by: Nitesh Konkar
---
Before Patch:
virsh cpu-stats --domain 24 --start 158
CPU158:
cpu_time 0.073570788 seconds
vcpu_time0.033277032 seconds
CPU159:
cpu_time 0.0 seconds
vcpu_time0.0 seconds
error: Failed to retrieve CPU statistics for domain 24
error: invalid argument: start_cpu 280 larger than maximum of 159
After Patch:
virsh cpu-stats --domain 24 --start 158
CPU158:
cpu_time 0.073570788 seconds
vcpu_time0.033277032 seconds
CPU159:
cpu_time 0.0 seconds
vcpu_time0.0 seconds
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index bb1ee1c..7599bdc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7331,7 +7331,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
if (show_count < 0 || show_count > max_id) {
if (show_count > max_id)
vshPrint(ctl, _("Only %d CPUs available to show\n"), max_id);
-show_count = max_id;
+show_count = max_id - cpu;
}
/* get percpu information */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I pushed the rc2 tag in git head and signed tarball and rpms to the
usual place:
ftp://libvrt.org/libvirt/
I didn't really had time to test it TBH, seems we don't have too much red
in the CI https://ci.centos.org/view/libvirt-project/ , mostly perl tests,
and virt manager, better than a few days ago !
Please give it more testing and hopefully I can make a release this w.e.
or early monday,
thanks !
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 16/3/31 上午5:47, John Ferlan wrote:
On 03/28/2016 12:02 AM, Rudy Zhang wrote:
Block migration speed is differect from memory migration speed, because
it not convert speed from Mb/sec to bytes/sec in the drive-mirror job.
"different"
Perhaps better stated:
Commit id '7b7600b3' added qemuMigrationDriveMirror to handle NBD
mirroring, but passed the migrate_speed listed in MiB/s to be used for
the mirror_speed which expects a bytes/s value.
This patch will convert the migrate_speed value to its mirror_speed
equivalent.
It's better. Thank you.
Signed-off-by: Rudy Zhang
---
src/qemu/qemu_migration.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Not my area of expertise, but since it was sitting here waiting for
review. Had to dig a bit, but yes I see the passed 'speed' is the
migration speed which yes, is in MiB/s...
What I didn't dig on was whether the migrate bandwidth using a negative
value (eg essentially unlimited) is passed around as a -1 or some other
theoretically maximum. If it is, then the bit shift done here is going
to have a problem as the value passed to qemuMonitorDriveMirror won't be
right (look at the qemuDomainBlockRebase and see how it limits things).
bandwidth will not be negative value.
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8bc76bf..7648d8c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2135,8 +2135,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
My suggested adjustments:
1. Adjust name of parameter "speed" to "migrate_speed" and also comments
into function to indicate new name and that it's MiB/s.
2. Add a local "unsigned long long mirror_speed = migrate_speed;"
3. Look at qemuDomainBlockRebase and how it checks to make sure the
speed will fit when shifted... Not sure we want to error out at this
point, but some thing like:
if (mirror_speed > LLONG_MAX >> 20)
mirror_speed = 0; /* means unlimited */
else
mirror_speed <<= 20;
It is usefull, but speed can't be '0', it need return error to callers.
qemuBlockJobSyncBegin(disk);
/* Force "raw" format for NBD export */
-mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,
- "raw", speed, 0, 0, mirror_flags);
4. Change the following to use "mirror_speed", I think for alignment
stuff it'll be best to be:
mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, "raw",
mirror_speed, 0, 0, mirror_flags);
Of course I could be totally off base and we just choose to use
mirror_speed = 0 and forget all the adjustments and we won't need to
pass migrate_speed either!
John
+mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,"raw",
+(unsigned long long)speed << 20, 0, 0, mirror_flags);
VIR_FREE(diskAlias);
VIR_FREE(nbd_dest);
I will change into patch v2, thank you for your reply.
--
Best regards,
Rudy Zhang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/31/2016 05:22 AM, Vasiliy Tolstov wrote:
Allow to use ip address and routes elements inside network ethernet.
Also allow to configure point-to-point address for host side device.
In order to allow for possible backporting of parts of this feature
without requiring backports of the rest, I think this should be in 4
patches:
1) changes to virnetdev.[ch] to support setting a peer address
2) changes to domain_conf.[ch], domaincommon.rng (that's the RNG file
that needs modification, not network.rng), and formatdomain.html.in to
add peer address to the config.
3) change to lxc_container.c to support setting the peer address.
4) change to qemu_interface.c to support setting ip/peer/route.
This way someone could backport the lxc support without needing to
backport qemu support, or they could backport some potential future fix
to one of the virNetDev*() functions without needing to backport the new
functionality at the hypervisor level. (I know this seems pedantic, but
multiple times I've ended up needing to manaually mangle bugfix patches
during a backport in order to account for lack of some feature on the
old branch).
Signed-off-by: Vasiliy Tolstov
---
docs/formatdomain.html.in| 12 -
docs/schemas/network.rng | 3 +++
As I noted above, the file that needs this addition is domaincommon.rng
(which has the RNG for domain XML) not network.rng (which has the RNG
for libvirt virtual network XML, a completely different thing from the
inside a ).
include/libvirt/libvirt-domain.h | 1 +
src/conf/domain_conf.c | 14 ++-
src/conf/domain_conf.h | 1 +
src/conf/network_conf.c | 26 ++-
src/conf/network_conf.h | 1 +
Since it seems that my understanding is correct (this feature is to
support setting a POINTOPOINT peer address on the tap device used by a
domain's device), you should get rid of the
changes to network_conf.[ch] - those are for configuring a libvirt
virtual network, not for configuring a domain's element.
src/lxc/lxc_container.c | 2 +-
src/network/bridge_driver.c | 2 +-
The change to bridge_driver should also be eliminated (except that
you'll need to put a NULL in place of the peer address). AFAIK it's
nonsensical to set a POINTOPOINT peer address for a linux host bridge
device (that's what this change does).
src/qemu/qemu_interface.c| 39 +
src/util/virnetdev.c | 54
src/util/virnetdev.h | 1 +
12 files changed, 135 insertions(+), 21 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 71ffe750452e..1203e96a8286 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4717,6 +4717,7 @@ qemu-kvm -net nic,model=? /dev/null
+
@@ -4749,7 +4750,16 @@ qemu-kvm -net nic,model=? /dev/null
to define the network routes to use for the network device. The attributes
of this element are described in the documentation for the
route
element in network
definitions.
-This is only used by the LXC driver.
+This is used by the LXC driver and Since 1.3.3
by the QEMU
+driver.
+
+
+
+Since 1.3.3 ip elements can hold peer attribute
to assign
+point-to-point address for the network device. The attributes of this
element
"to assign *a* point-to-point address"
+are described in the documentation for the ip element in
+network definitions.
The above is untrue. You haven't made (and *shouldn't* make) any changes
to formatnetwork.html.in, so it doesn't contain any info about the
"peer" attribute. Since there are differences between for a domain
and for a network, you should document them separately.
+This is only used by the LXC and QEMU driver.
s/driver/drivers/
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7e06796c3c73..437e87fac01c 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3937,6 +3937,7 @@ typedef virDomainIPAddress *virDomainIPAddressPtr;
struct _virDomainInterfaceIPAddress {
int type;/* virIPAddrType */
char *addr; /* IP address */
+char *peer; /* IP peer */
unsigned int prefix; /* IP address prefix */
};
You've added this into the struct that is used to report IP address info
returned by virDomainInterfaceAddresses, but haven't put anything into
the code supporting that API to report a peer add
On Thu, Mar 31, 2016 at 07:49:10PM +0200, Erik Skultety wrote:
> We should encourage libvirt users to use our own constants when modifying
> logging level, instead of their own values.
> ---
> include/libvirt/libvirt-admin.h | 15 +++
> src/util/virlog.h | 10 --
> 2 files changed, 15 insertions(+), 10 deletions(-)
If we don't expose API for changing the global log level, we can
avoid exposing these constants.
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:49:04PM +0200, Erik Skultety wrote:
> Enable libvirt users to query for the current logging level setting.
I'm not really convinced we need to expose this via the API,
as the 'log_level' flag has really long ago ceased to be something
practical to use. We generate soo much debug information
these days that you just drown in noise if you set log_level.
In fact, I'd suggest we should probably just deprecate it as
a concept, with a view to removing it entirely in future.
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:49:02PM +0200, Erik Skultety wrote:
> If the API isn't closed, a situation with 2 setters where one is about to
> define a set of outputs and the other is already defining a new one, may
> occur.
> By resetting all outputs, all file descriptors are closed. However, the other
> setter may still have a dangling reference to a file descriptor which may have
> already been closed.
> ---
> src/libvirt_private.syms | 2 ++
> src/util/virlog.c| 15 +++
> src/util/virlog.h| 2 ++
> 3 files changed, 19 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index cc40b46..14047f5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner;
>
>
> # util/virlog.h
> +virLogAPILock;
> +virLogAPIUnlock;
> virLogDefineFilters;
> virLogDefineOutputs;
> virLogFilterListFree;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 769dcec..6aa9c91 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src,
> void *data);
>
>
> +/* Setters need to be serialized on API entry point */
> +static virMutex virLogAPIMutex;
> +
> +void
> +virLogAPILock(void)
> +{
> +virMutexLock(&virLogAPIMutex);
> +}
> +
> +void
> +virLogAPIUnlock(void)
> +{
> +virMutexUnlock(&virLogAPIMutex);
> +}
> +
> /*
> * Logs accesses must be serialized though a mutex
> */
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index 1c55a48..f5c0a4f 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list,
> int count);
> * Internal logging API
> */
>
> +extern void virLogAPILock(void);
> +extern void virLogAPIUnlock(void);
> extern void virLogLock(void);
> extern void virLogUnlock(void);
I'm not really seeing the reason why we need a second mutex instead of
just ensuring we acquire the existing mutex in the right places.
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/31/2016 05:23 AM, Vasiliy Tolstov wrote:
2016-03-23 20:46 GMT+03:00 Laine Stump :
Since there is no documentation included with the patch, and the wrong RNG
file has been modified, I'm not clear on exactly why a libvirt virtual
network would use a peer address.
Normally libvirt networks are made by creating a bridge device, adding in
some iptables rules, and running an instance of dnsmasq to service dhcp and
dns requests made by guests who have tap devices connected to that network.
But if I understand correctly, your patches are intended to allow setting
the local and peer address for guest-connected tap devices that aren't
attached to a bridge on the host side, but instead rely on the host's IP
stack to route appropriate traffic through the tap device. If so, then why
is a libvirt network involved at all? Why/how could a bridge device be used
for a point-to-point link? If this isn't just a misunderstanding of which
parts of libvirt code affect what, then some examples (and patches to
formatdomain.html.in/formatnetwork.html.in) would be very useful to help me
understand.
I'm send new patch with some docs in formatdomain.
Sometimes bridges not allowed or not needed. We use plain tap devices
on host side and bird routing daemon to route traffic to/from tap
devices.
But the changes to network_conf.c, network_conf.h, and bridge_driver.c
only serve to assign a POINTOPOINT IP address pair to *the bridge* of a
libvirt-managed network. This is unrelated to any IP address pair
assigned to a tap device used by an lxc or qemu domain.
I'll comment further in your new patch.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
If the API isn't closed, a situation with 2 setters where one is about to
define a set of outputs and the other is already defining a new one, may occur.
By resetting all outputs, all file descriptors are closed. However, the other
setter may still have a dangling reference to a file descriptor which may have
already been closed.
---
src/libvirt_private.syms | 2 ++
src/util/virlog.c| 15 +++
src/util/virlog.h| 2 ++
3 files changed, 19 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cc40b46..14047f5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1741,6 +1741,8 @@ virLockSpaceReleaseResourcesForOwner;
# util/virlog.h
+virLogAPILock;
+virLogAPIUnlock;
virLogDefineFilters;
virLogDefineOutputs;
virLogFilterListFree;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 769dcec..6aa9c91 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -128,6 +128,21 @@ static void virLogOutputToFd(virLogSourcePtr src,
void *data);
+/* Setters need to be serialized on API entry point */
+static virMutex virLogAPIMutex;
+
+void
+virLogAPILock(void)
+{
+virMutexLock(&virLogAPIMutex);
+}
+
+void
+virLogAPIUnlock(void)
+{
+virMutexUnlock(&virLogAPIMutex);
+}
+
/*
* Logs accesses must be serialized though a mutex
*/
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 1c55a48..f5c0a4f 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -203,6 +203,8 @@ extern void virLogFilterListFree(virLogFilterPtr *list, int
count);
* Internal logging API
*/
+extern void virLogAPILock(void);
+extern void virLogAPIUnlock(void);
extern void virLogLock(void);
extern void virLogUnlock(void);
extern int virLogReset(void);
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
The source update is only triggered when a logging filter gets changed (by
incrementing virLogSerial counter), but if we also want to enable remote
global priority tuning, any change to priority needs to trigger the source
update as well.
---
src/util/virlog.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index a837e17..769dcec 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -265,6 +265,7 @@ virLogSetDefaultPriority(virLogPriority priority)
return -1;
virLogDefaultPriority = priority;
+virLogSerial++;
return 0;
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This API is the entry point to output modification of the logger. Currently,
everything is done by virLogParseOutputs which should do parsing only and
a separate method which will do the definition part should be added.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 23 +++
src/util/virlog.h| 1 +
3 files changed, 25 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bd3f72d..99b3b55 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1759,6 +1759,7 @@ virLogReset;
virLogSetDefaultPriority;
virLogSetFilters;
virLogSetFromEnv;
+virLogSetOutputs;
virLogUnlock;
virLogVMessage;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 0aa722d..2cd3773 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1516,3 +1516,26 @@ virLogSetFilters(const char *filters)
return ret;
}
+
+
+/**
+ * virLogSetOutputs:
+ * @outputs: string defining a (set of) output(s)
+ *
+ * Replaces the current set of defined outputs with a new set of outputs.
+ *
+ * Returns the number of outputs successfully defined or -1 in case of an
+ * error.
+ */
+int
+virLogSetOutputs(const char *outputs)
+{
+int ret = -1;
+
+if (virLogInitialize() < 0)
+return -1;
+
+ret = virLogParseOutputs(outputs);
+
+return ret;
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 4d7cc98..4141ecd 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -184,6 +184,7 @@ extern virLogPriority virLogGetDefaultPriority(void);
extern int virLogSetDefaultPriority(virLogPriority priority);
extern void virLogSetFromEnv(void);
extern int virLogSetFilters(const char *filters);
+extern int virLogSetOutputs(const char *outputs);
extern int virLogDefineFilter(const char *match,
virLogPriority priority,
unsigned int flags);
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This is just a convenience method for discarding a list of outputs instead of
using a 'for' loop everywhere. It is safe to pass -1 as the number of elements
in the list as well as passing NULL as list reference.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 20
src/util/virlog.h| 1 +
3 files changed, 22 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 433cf60..e594eaa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1752,6 +1752,7 @@ virLogGetOutputs;
virLogLock;
virLogMessage;
virLogOutputFree;
+virLogOutputListFree;
virLogOutputNew;
virLogParseDefaultPriority;
virLogParseFilters;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index e36ff73..5da1af7 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1600,3 +1600,23 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t
nfilters)
return virLogNbFilters;
}
+
+/**
+ * virLogOutputsFreeList:
+ * @list: list of outputs to be freed
+ * @count: number of elements in the list
+ *
+ * Frees a list of outputs.
+ */
+void
+virLogOutputListFree(virLogOutputPtr *list, int count)
+{
+size_t i;
+
+if (!list || count < 0)
+return;
+
+for (i = 0; i < count; i++)
+virLogOutputFree(list[i]);
+VIR_FREE(list);
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 7573984..4f0eea7 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -196,6 +196,7 @@ extern int virLogOutputNew(virLogOutputFunc f,
const char *name,
unsigned int flags);
extern void virLogOutputFree(virLogOutputPtr output);
+extern void virLogOutputListFree(virLogOutputPtr *list, int count);
/*
* Internal logging API
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This is just a convenience method for discarding a list of filters instead of
using a 'for' loop everywhere. It is safe to pass -1 as the number of elements
in the list as well as passing NULL as list reference.
---
src/util/virlog.c | 20
src/util/virlog.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 36fecda..a1f5872 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1630,3 +1630,23 @@ virLogFilterFree(virLogFilterPtr filter)
VIR_FREE(filter->match);
VIR_FREE(filter);
}
+
+/**
+ * virLogFilterFreeList:
+ * @list: list of filters to be freed
+ * @count: number of elements in the list
+ *
+ * Frees a list of filters.
+ */
+void
+virLogFilterListFree(virLogFilterPtr *list, int count)
+{
+size_t i;
+
+if (!list || count < 0)
+return;
+
+for (i = 0; i < count; i++)
+virLogFilterFree(list[i]);
+VIR_FREE(list);
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index eec3a5d..eca7894 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -198,6 +198,7 @@ extern int virLogOutputNew(virLogOutputFunc f,
extern void virLogOutputFree(virLogOutputPtr output);
extern void virLogFilterFree(virLogFilterPtr filter);
extern void virLogOutputListFree(virLogOutputPtr *list, int count);
+extern void virLogFilterListFree(virLogFilterPtr *list, int count);
/*
* Internal logging API
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Actually perform the swap between the active set of outputs and the
user-provided copy. Deallocation ensures that no file descriptors that should
not have been closed are not closed and all descriptors that should be closed
are closed properly.
---
src/util/virlog.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index a20cde4..8e14a9e 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1641,22 +1641,29 @@ virLogSetOutputs(const char *outputs)
* @outputs: new set of outputs to be defined
* @noutputs: number of outputs in @outputs
*
- * Resets any existing set of outputs and defines a completely new one.
+ * Swaps any existing set of outputs with a completely new one.
*
* Returns number of outputs successfully defined or -1 in case of error;
*/
int
virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
{
+size_t count;
+virLogOutputPtr *tmp = NULL;
+
if (virLogInitialize() < 0)
return -1;
virLogLock();
-virLogResetOutputs();
+/* swap the currently defined list with a new copy and */
+tmp = virLogOutputs;
+count = virLogNbOutputs;
virLogOutputs = outputs;
virLogNbOutputs = noutputs;
virLogUnlock();
+virLogOutputListFree(tmp, count);
+
return virLogNbOutputs;
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Outputs are a little bit trickier than filters, because if user wants to change
the set of outputs that is currently defined for a different set that may
contain outputs that do already exist in the current set, opening a file
descriptor for that output would only work for files, but not for syslog or
journald, because that would overwrite the existing ones which might (and
probably will be in use by workers). So, we need a mechanism to find out, if
such an output is already defined and only copy its data. For files, the data
also include the already opened file descriptor. For journald, the file
descriptor is global, so nothing to copy there. Syslog is the most trickiest
one, since it keeps the open file descriptor to system daemon private and
cannot be reopened (to change the message prefix) until the very last moment
when the whole output set is ready to be swapped with the defined one.
This method is also used when deallocating a list of outputs (either the new
copy that is just being prepared or the global active set of outputs).
According to the paragraph above, the problem of changing data when it is
being used has been solved. It still needs to be ensured that when deallocating
the global active set of outputs after it was swap with the user-provided copy
does not close all file descriptors, since that would cause invalid references
in the copy that we just swapped. By issuing virLogOutputExist during
deallocation, the described scenario can be safely prevented.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 36
src/util/virlog.h| 1 +
3 files changed, 38 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 319ef18..cc40b46 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1752,6 +1752,7 @@ virLogGetNbOutputs;
virLogGetOutputs;
virLogLock;
virLogMessage;
+virLogOutputExists;
virLogOutputFree;
virLogOutputListFree;
virLogOutputNew;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 9fe47fb..204372f 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -186,6 +186,42 @@ VIR_ONCE_GLOBAL_INIT(virLog)
/**
+ * virLogOutputExists:
+ * @dest: destination type
+ * @opaque: opaque data to the method
+ *
+ * Looks for an output of destination type @dest. If such output exists,
+ * a reference to the output is returned, unless the destination is of type
+ * file in which case a comparison of the output's data with @opaque needs to
+ * be done first.
+ *
+ * Returns a reference to an output if one was found or NULL if such output
+ * does not exist.
+ */
+virLogOutputPtr
+virLogOutputExists(virLogDestination dest, const void *opaque)
+{
+size_t i;
+const char *name = opaque;
+
+for (i = 0; i < virLogNbOutputs; i++) {
+if (!virLogOutputs[i])
+continue;
+
+if (dest == virLogOutputs[i]->dest) {
+if (dest != VIR_LOG_TO_FILE)
+return virLogOutputs[i];
+
+if (STREQ(virLogOutputs[i]->name, name))
+return virLogOutputs[i];
+}
+}
+
+return NULL;
+}
+
+
+/**
* virLogReset:
*
* Reset the logging module to its default initial state
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 0102489..1c55a48 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -229,5 +229,6 @@ extern void virLogVMessage(virLogSourcePtr source,
bool virLogProbablyLogMessage(const char *str);
int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs);
int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters);
+virLogOutputPtr virLogOutputExists(virLogDestination dest, const void *opaque);
#endif
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This method will eventually replace the existing virLogParseFilters which
currently does both parsing and defining. However, each method should do one
thing only, so will the current virLogParseFilters method will be split into
parsing and defining, only to be swallowed by the entry-point API
virLogSetFilters.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 23 +++
src/util/virlog.h| 1 +
3 files changed, 25 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 684f06c..bd3f72d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1757,6 +1757,7 @@ virLogPriorityFromSyslog;
virLogProbablyLogMessage;
virLogReset;
virLogSetDefaultPriority;
+virLogSetFilters;
virLogSetFromEnv;
virLogUnlock;
virLogVMessage;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index b893365..0aa722d 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1493,3 +1493,26 @@ bool virLogProbablyLogMessage(const char *str)
ret = true;
return ret;
}
+
+
+/**
+ * virLogSetFilters:
+ * @filters: string defining a (set of) filter(s)
+ *
+ * Replaces the current set of defined filters with a new set of filters.
+ *
+ * Returns the number of filters successfully defined or -1 in case of an
+ * error.
+ */
+int
+virLogSetFilters(const char *filters)
+{
+int ret = -1;
+
+if (virLogInitialize() < 0)
+return -1;
+
+ret = virLogParseFilters(filters);
+
+return ret;
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 36c610b..4d7cc98 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -183,6 +183,7 @@ extern char *virLogGetOutputs(void);
extern virLogPriority virLogGetDefaultPriority(void);
extern int virLogSetDefaultPriority(virLogPriority priority);
extern void virLogSetFromEnv(void);
+extern int virLogSetFilters(const char *filters);
extern int virLogDefineFilter(const char *match,
virLogPriority priority,
unsigned int flags);
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Prepare a method that only defines a set of filters. It takes a list of
filters, preferably created by virLogParseFilters. The original set of filters
is reset and replaced by the new user-provided set of filters.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 25 +
src/util/virlog.h| 1 +
3 files changed, 27 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e97ec45..8773270 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1742,6 +1742,7 @@ virLockSpaceReleaseResourcesForOwner;
# util/virlog.h
virLogDefineFilter;
+virLogDefineFilters;
virLogDefineOutput;
virLogDefineOutputs;
virLogGetDefaultPriority;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index e5923ce..60573a9 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1564,3 +1564,28 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t
noutputs)
return virLogNbOutputs;
}
+
+/**
+ * virLogDefineFilters:
+ * @filters: new set of filters to be defined
+ * @nfilters: number of filters in @filters
+ *
+ * Resets any existing set of filters and defines a completely new one.
+ *
+ * Returns number of filters successfully defined or -1 in case of error;
+ */
+int
+virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
+{
+if (virLogInitialize() < 0)
+return -1;
+
+virLogLock();
+virLogResetOutputs();
+virLogFilters = filters;
+virLogNbOutputs = nfilters;
+virLogFiltersSerial++;
+virLogUnlock();
+
+return virLogNbFilters;
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 166db5a..ff16034 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -225,5 +225,6 @@ extern void virLogVMessage(virLogSourcePtr source,
bool virLogProbablyLogMessage(const char *str);
int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs);
+int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters);
#endif
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In this particular case, reset is meant as clearing the whole list of
outputs/filters, not resetting it to a predefined default setting. Looking at
it from that perspective, returning the number of records removed doesn't help
the caller in any way (not that any of the callers would actually check for
it). Well, callers could detect an error from the number of successfully
removed records, but the only thing that can fail in virLogReset is force
closing a file descriptor in which case the error isn't propagated back to
virLogReset anyway. Conclusion: there is no practical use for having a return
type of 'int' rather than 'void' in this case.
---
src/util/virlog.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 738eaac..6d11328 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -117,8 +117,8 @@ static int virLogNbOutputs;
*/
static virLogPriority virLogDefaultPriority = VIR_LOG_DEFAULT;
-static int virLogResetFilters(void);
-static int virLogResetOutputs(void);
+static void virLogResetFilters(void);
+static void virLogResetOutputs(void);
static void virLogOutputToFd(virLogSourcePtr src,
virLogPriority priority,
const char *filename,
@@ -239,10 +239,8 @@ virLogSetDefaultPriority(virLogPriority priority)
* virLogResetFilters:
*
* Removes the set of logging filters defined.
- *
- * Returns the number of filters removed
*/
-static int
+static void
virLogResetFilters(void)
{
size_t i;
@@ -252,7 +250,6 @@ virLogResetFilters(void)
VIR_FREE(virLogFilters);
virLogNbFilters = 0;
virLogFiltersSerial++;
-return i;
}
@@ -319,10 +316,8 @@ virLogDefineFilter(const char *match,
* virLogResetOutputs:
*
* Removes the set of logging output defined.
- *
- * Returns the number of output removed
*/
-static int
+static void
virLogResetOutputs(void)
{
size_t i;
@@ -333,9 +328,7 @@ virLogResetOutputs(void)
VIR_FREE(virLogOutputs[i].name);
}
VIR_FREE(virLogOutputs);
-i = virLogNbOutputs;
virLogNbOutputs = 0;
-return i;
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Prepare a method that only defines a set of outputs. It takes a list of
outputs, preferably created by virLogParseOutputs. The original set of outputs
is reset and replaced by the new user-provided set of outputs.
---
src/libvirt_private.syms | 1 +
src/util/virlog.c| 25 +
src/util/virlog.h| 1 +
3 files changed, 27 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 99b3b55..e97ec45 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1743,6 +1743,7 @@ virLockSpaceReleaseResourcesForOwner;
# util/virlog.h
virLogDefineFilter;
virLogDefineOutput;
+virLogDefineOutputs;
virLogGetDefaultPriority;
virLogGetFilters;
virLogGetNbFilters;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 9059722..e5923ce 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1539,3 +1539,28 @@ virLogSetOutputs(const char *outputs)
return ret;
}
+
+
+/**
+ * virLogDefineOutputs:
+ * @outputs: new set of outputs to be defined
+ * @noutputs: number of outputs in @outputs
+ *
+ * Resets any existing set of outputs and defines a completely new one.
+ *
+ * Returns number of outputs successfully defined or -1 in case of error;
+ */
+int
+virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
+{
+if (virLogInitialize() < 0)
+return -1;
+
+virLogLock();
+virLogResetOutputs();
+virLogOutputs = outputs;
+virLogNbOutputs = noutputs;
+virLogUnlock();
+
+return virLogNbOutputs;
+}
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 4141ecd..166db5a 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -224,5 +224,6 @@ extern void virLogVMessage(virLogSourcePtr source,
va_list vargs) ATTRIBUTE_FMT_PRINTF(7, 0);
bool virLogProbablyLogMessage(const char *str);
+int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs);
#endif
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Start using the method that was introduced and described by commit 29d0068b.
---
src/util/virlog.c | 83 ++-
1 file changed, 58 insertions(+), 25 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 204372f..62533b1 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -344,10 +344,17 @@ virLogResetOutputs(void)
void
virLogOutputFree(virLogOutputPtr output)
{
+virLogOutputPtr tmp = NULL;
+
if (!output)
return;
-if (output->c)
+/* @output shall be closed only if such output does not already exist
+ * in the global list of outputs or @output itself is part of the global
+ * list of outputs
+ */
+tmp = virLogOutputExists(output->dest, output->name);
+if (output->c && (!tmp || tmp == output))
output->c(output->data);
VIR_FREE(output->name);
VIR_FREE(output);
@@ -733,16 +740,21 @@ virLogNewOutputToFile(virLogPriority priority,
const char *file)
{
int fd;
-virLogOutputPtr ret = NULL;
+virLogOutputPtr gref, ret = NULL;
-fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR);
-if (fd < 0)
-return NULL;
+if ((gref = virLogOutputExists(VIR_LOG_TO_FILE, file))) {
+fd = (intptr_t) gref->data;
+} else {
+fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR);
+if (fd < 0)
+ return NULL;
+}
if (!(ret = virLogOutputNew(virLogOutputToFd, virLogCloseFd,
(void *)(intptr_t)fd,
priority, VIR_LOG_TO_FILE, file))) {
-VIR_LOG_CLOSE(fd);
+if (!gref)
+VIR_LOG_CLOSE(fd);
return NULL;
}
return ret;
@@ -818,21 +830,39 @@ static virLogOutputPtr
virLogNewOutputToSyslog(virLogPriority priority,
const char *ident)
{
-virLogOutputPtr ret = NULL;
-
-/*
- * ident needs to be kept around on Solaris
+virLogOutputPtr gref, ret = NULL;
+
+/* syslog suffers from several issues:
+ * 1) Every other call to openlog would be a NOOP, but we can't close the
+ * existing connection just yet, because we're not holding the lock and
+ * the output cannot be changed until we're sure nothing can go wrong and
+ * we just swap our new list of outputs with the old global one.
+ *
+ * 2) Syslog keeps the open file descriptor private, so we can't just copy
+ * it like we do it with files if an output to syslog already exists and
+ * even if it did, there is no other way than to issue openlog if user
+ * wants to change ident.
+ *
+ * Therefore, dealing with syslog has to be special-cased and postponed
+ * until the very last moment.
*/
-VIR_FREE(current_ident);
-if (VIR_STRDUP(current_ident, ident) < 0)
-return NULL;
+if (!(gref = virLogOutputExists(VIR_LOG_TO_SYSLOG, NULL))) {
+/*
+ * rather than copying @ident, syslog uses caller's reference instead
+ */
+VIR_FREE(current_ident);
+if (VIR_STRDUP(current_ident, ident) < 0)
+return NULL;
+
+openlog(current_ident, 0, 0);
+}
-openlog(current_ident, 0, 0);
if (!(ret = virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog,
-NULL, priority, VIR_LOG_TO_SYSLOG,
-ident))) {
-closelog();
-VIR_FREE(current_ident);
+NULL, priority, VIR_LOG_TO_SYSLOG, ident))) {
+if (!gref) {
+closelog();
+VIR_FREE(current_ident);
+}
return NULL;
}
return ret;
@@ -1043,19 +1073,22 @@ static void virLogCloseJournald(void *data
ATTRIBUTE_UNUSED)
static virLogOutputPtr virLogNewOutputToJournald(int priority)
{
-virLogOutputPtr ret = NULL;
+virLogOutputPtr gref, ret = NULL;
-if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0)
-return NULL;
-if (virSetInherit(journalfd, false) < 0) {
-VIR_LOG_CLOSE(journalfd);
-return NULL;
+if (!(gref = virLogOutputExists(VIR_LOG_TO_JOURNALD, NULL))) {
+if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0)
+return NULL;
+if (virSetInherit(journalfd, false) < 0) {
+VIR_LOG_CLOSE(journalfd);
+return NULL;
+}
}
if (!(ret = virLogOutputNew(virLogOutputToJournald,
virLogCloseJournald, NULL,
priority, VIR_LOG_TO_JOURNALD, NULL))) {
-VIR_LOG_CLOSE(journalfd);
+if (!gref)
+VIR_LOG_CLOSE(journalfd);
return NULL;
}
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This series implements administration APIs to modify daemon's logging settings
which include priority, filters, and outputs. It also performs all necessary
changes to enable this feature. Patches also involve some slight refactors
to the existing code as well as wiring up the APIs to virt-admin client.
The major change in internal code is how definition of filters and outputs is
achieved. Until now each filter/output has been defined and appended to the
global set of filters/outputs individually whilst holding a lock that was
repeatedly released after each filter/output. If modification to filters and
outputs should be exported through public APIs, all these setter operations
must be atomic, i.e. accomplished as a unit, not gradually. For filters and
priority, these are replaced normally, i.e. reset the original and replace it
with the new copy while having the lock. For outputs, there was a slight issue,
since resetting the global set of outputs would result in closing all the file
descriptors, which might not be exactly what we want, since the new set of
outputs that is meant to replace the original set might include outputs that
are also contained within the original set. The logic for such outputs is
following:
Each file-based output that already exists in the original set and
should continue existing in the new set will copy the existing FD from the
global set. For each syslog-based output, closelog and openlog must be called
anyway (because syslog keeps its FD private), but will be called at
the very last moment if such an output already exists to prevent from changing
before the lock for unit replaced is acquired, and thus breaking the atomicity
that we want. For journald-based outputs, the FD is global, we just need to
prevent it from being closed if such an output shall continue to exist.
All outputs that need to continue to exist are prevented from closing when
being destroyed-deallocated. On the other hand, all outputs that do not exist
in the new set of outputs shall be closed correctly.
Erik Skultety (38):
virlog: Return void instead of int in virLogReset methods
virlog: Convert virLogOutputs to a list of pointers to outputs
virlog: Convert virLogFilters to a list of pointers to filters
virlog: Export virLogOutputPtr through header
virlog: Export virLogFilterPtr through header
virlog: Introduce virLogSetFilters
virlog: Introduce virLogSetOutputs
daemon: Replace virLogParseOutputs with virLogSetOutputs in callers
daemon: Replace virLogParseFilters with virLogSetFilters in callers
virlog: Introduce virLogDefineOutputs
virlog: Introduce virLogDefineFilters
virlog: Rename virLogAddOutputTo to virLogNewOutput
virlog: Rename virLogDefineOutput to virLogOutputNew
virlog: Rename virLogDefineFilter to virLogFilterNew
virlog: Introduce virLogOutputFree
virlog: Introduce virLogOutputListFree
virlog: Introduce virLogFilterFree
virlog: Introduce virLogFilterListFree
virlog: Make virLogReset methods use of virLog(Output|Filter)ListFree
virlog: Split output parsing and output defining to separate
operations
virlog: Split filter parsing and filter defining to separate
operations
virlog: Split parsing and setting priority
virlog: Introduce virLogOutputExists
virlog: Make use of virLogOutputExists
virlog: Take a special care of syslog when setting new set of log
outputs
virlog: Swap the new copy of outputs with the global existing one
virlog: Rename virLogFiltersSerial to virLogSerial
virlog: Make virLogSetDefaultPriority trigger source update as well
virlog: Introduce an API mutex that serializes all setters
virlog: Acquire virLogAPILock in each setter API
admin: Introduce virAdmConnectGetLoggingLevel
admin: Introduce virAdmConnectGetLoggingFilters
admin: Introduce virAdmConnectGetLoggingOutputs
admin: Introduce virAdmConnectSetLoggingLevel
admin: Introduce virAdmConnectSetLoggingFilters
admin: Introduce virAdmConnectSetLoggingOutputs
admin: Export logging level constants via libvirt-admin.h
virt-admin: Wire-up the logging APIs
daemon/admin.c | 125 +++
daemon/libvirtd.c | 8 +-
include/libvirt/libvirt-admin.h | 37 +++
src/admin/admin_protocol.x | 73 -
src/admin/admin_remote.c| 86 +
src/admin_protocol-structs | 38 +++
src/libvirt-admin.c | 219 +
src/libvirt_admin_private.syms | 9 +
src/libvirt_admin_public.syms | 6 +
src/libvirt_private.syms| 14 +-
src/locking/lock_daemon.c | 8 +-
src/logging/log_daemon.c| 8 +-
src/util/virlog.c | 698 +++-
src/util/virlog.h | 50 +--
tests/eventtest.c | 3 +-
tests/testutils.c | 19 +-
tests/virlogtest.c | 12 +-
tools/virt-admin.c | 208
18 files changed, 1361 insertions(+), 260 deletions(-)
--
2.4.3
--
libvir-list
On Mon, Mar 28, 2016 at 12:02:09PM +0800, Rudy Zhang wrote:
> Block migration speed is differect from memory migration speed, because
> it not convert speed from Mb/sec to bytes/sec in the drive-mirror job.
>
It might be worth noting that this was introduced by commit 08cc14f [1],
released in libvirt 1.2.9, which removed the conversion from
qemuMonitorDriveMirror but did not adjust all the callers.
> Signed-off-by: Rudy Zhang
> ---
> src/qemu/qemu_migration.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 8bc76bf..7648d8c 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2135,8 +2135,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
>
> qemuBlockJobSyncBegin(disk);
> /* Force "raw" format for NBD export */
> -mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,
> - "raw", speed, 0, 0, mirror_flags);
> +mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias,
> nbd_dest,"raw",
> +(unsigned long long)speed << 20, 0, 0, mirror_flags);
This could overflow if sizeof(unsigned long) == sizeof(unsigned long
long), but we don't seem to mind taking the bandwith passed by
virTypedParams in an unsigned long long type and passing it to
qemuMigrationRun as unsigned long.
This comment of qemuMigrationDriveMirror:
* @speed: how much should the copying be limited
could also be ajdusted to include the units.
I would push the patch but I do not have the time to try it out now.
Jan
[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=08cc14f
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 03:07:25PM +0800, zhukaijie wrote:
> Hello,Each running VM has it's command line logged in
> /var/log/libvirt/qemu/${vmname}.And now I'd like to disable this log
> function, that is to say, not to recordthe QEMU command line of VM.
> Could you please tell me will can I configure it? Are there any configure
> info such as "log level" to coordinate this log function? In fact, once
> the qemu command line contains some sensitive info which I try to prevent
> others from see it, I can make the qemu command not to be logged.
There's no need to resend the same message to multiple lists, particularly
after I already answered on the previous posting.
The log file is only readable by root, and there's nothing sensitive in
there that root doesn't already have access to view.
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hello,Each running VM has it's command line logged in
/var/log/libvirt/qemu/${vmname}.And now I'd like to disable this log function,
that is to say, not to recordthe QEMU command line of VM. Could you please tell
me will can I configure it? Are there any configure info such as "log level" to
coordinate this log function? In fact, once the qemu command line contains some
sensitive info which I try to prevent others from see it, I can make the qemu
command not to be logged.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Daniel P. Berrange wrote:
> On Wed, Mar 30, 2016 at 04:36:03PM -0600, Jim Fehlig wrote:
>> Chunyan sent a nice cleanup patch for libxlDomainDetachNetDevice
>>
>> https://www.redhat.com/archives/libvir-list/2016-March/msg00926.html
>>
>> which I incorrectly modified before pushing as commit b5534e53. My
>> modification caused network devices of type hostdev to no longer
>> be removed. This patch changes b5534e53 to resemble Chunyan's
>> original, correct patch.
>>
>> Signed-off-by: Jim Fehlig
>> ---
>> src/libxl/libxl_driver.c | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> ACK
Thanks. I've pushed both patches.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Thanks Laine,
Adding the driver_name o the config did the trick, thanks ☺
Regarding the vifo error it seem that openstack does roleback if operation
failed so that why you see the virDomainAttachDeviceFlags
Anyhow I found that the in qemu 2.1 suspend is not working (I got error [1] )
so I upgrade to qemu 2.5 and then suspend work but it failed on resume.
So Just to clarify vfio is not supporting “attach device” right? is qemu going
to support it?
Now I wonder if I need to hardcode in the hostdev config to be with
driver_name=kvm…
[1] -ERROR:qom/object.c:725:object_unref: assertion failed: (obj->ref > 0)
From: sendmail [mailto:justsendmailnothinge...@gmail.com] On Behalf Of Laine
Stump
Sent: Wednesday, March 30, 2016 9:25 PM
To: Libvirt
Cc: Moshe Levi
Subject: Re: [libvirt] Host device assignment driver name vfio/ kvm
On 03/29/2016 07:45 AM, Moshe Levi wrote:
Hi,
I was testing Host device assignment in OpenStack environment where the
driver name is vfio or kvm.
My setup is as follow:
1. Fedora 21
2. Libvirt 1.3.0 which I compiled
3. OpenStack master
I have also other setups with older Libvirt version and the same OpenStack
environment.
I notice that on my fedora environment the driver name is vfio were in my old
environment the driver name is kvm.
According to Libvirt documentation default is "vfio" on systems where the VFIO
driver is available and loaded, see [1]
I remove the vfio modules by removing vfio, vfio_iommu_type1, vfio_pci but when
I boot a vm the drive name is vfio
How can change the driver name to be kvm?
libvirt tries very hard to use vfio rather than legacy kvm, because legacy kvm
is old, deprecated, and "declared bad" :-). But it won't changed it to vfio if
you've explicitly said that you want to use kvm. If you really want to use
legacy kvm device assignment, manually set that in the config. When you do
that, if the system you're running on doesn't support it, it will error out
rather than switching.
Another thing that I encounter is an error when suspending VM (in OpenStack
environment) when the driver name is vfio.
In such case I am getting the following error from Libvirt:
2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher File
"/usr/lib64/python2.7/site-packages/libvirt.py", line 560, in attachDeviceFlags
2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher if ret ==
-1: raise libvirtError ('virDomainAttachDeviceFlags() failed', dom=self)
2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher libvirtError:
internal error: unable to execute QEMU command 'device_add': Device
initialization failed.
I would appreciate for some pointers on what can cause this issue.
Assuming that openstack uses libvirt's virDomainSave API I would expect
suspending a guest to fail if it had an assigned device (since libvirt
implements this by "migrating to disk", and qemu doesn't allow migration of a
guest with an assigned device. But your problem is that it's trying to *attach*
a device, which I wouldn't consider to be a part of a save or suspend or
whatever operation. Is it possible to get more information about what leads up
to this?
[1] https://libvirt.org/formatdomain.html#elementsHostDev
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/31/2016 11:58 AM, Moshe Levi wrote:
Thanks Laine,
Adding the driver_name o the config did the trick, thanks J
Regarding the vifo error it seem that openstack does roleback if
operation failed so that why you see the virDomainAttachDeviceFlags
Anyhow I found that the in qemu 2.1 suspend is not working (I got
error [1] ) so I upgrade to qemu 2.5 and then suspend work but it
failed on resume.
So Just to clarify vfio is not supporting “attach device” right? is
qemu going to support it?
Not at all. attach device works just fine with vfio (that's how hotplug
is done). I've just been using it myself in some testing. (note that
vfio has been supported since kernel 3.6 and libvirt 1.0.5, i.e. a "very
long time")
Now I wonder if I need to hardcode in the hostdev config to be with
driver_name=kvm…
I certainly hope not. You would be viciously attacked by the keeper of
vfio for your transgression! :-)
Seriously, there should be no reason whatsoever to force driver
name='kvm'. This was made configurable *only* as a fallback in case
errors were encountered with vfio during its early days. As a matter of
fact, I'm pretty sure that RHEL7 has legacy kvm device assignment
completely disabled. If you are needing to force legacy kvm device
assignment to make your setup work, then there is a bug somewhere that
needs to be investigated and squashed.
[1] -ERROR:qom/object.c:725:object_unref: assertion failed: (obj->ref > 0)
*From:*sendmail [mailto:justsendmailnothinge...@gmail.com] *On Behalf
Of *Laine Stump
*Sent:* Wednesday, March 30, 2016 9:25 PM
*To:* Libvirt
*Cc:* Moshe Levi
*Subject:* Re: [libvirt] Host device assignment driver name vfio/ kvm
On 03/29/2016 07:45 AM, Moshe Levi wrote:
Hi,
I was testing Host device assignment in OpenStack environment
where the driver name is vfio or kvm.
My setup is as follow:
1.Fedora 21
2.Libvirt 1.3.0 which I compiled
3.OpenStack master
I have also other setups with older Libvirt version and the same
OpenStack environment.
I notice that on my fedora environment the driver name is vfio
were in my old environment the driver name is kvm.
According to Libvirt documentation default is "vfio" on systems
where the VFIO driver is available and loaded, see [1]
I remove the vfio modules by removing vfio, vfio_iommu_type1,
vfio_pci but when I boot a vm the drive name is vfio
How can change the driver name to be kvm?
libvirt tries very hard to use vfio rather than legacy kvm, because
legacy kvm is old, deprecated, and "declared bad" :-). But it won't
changed it to vfio if you've explicitly said that you want to use kvm.
If you really want to use legacy kvm device assignment, manually set
that in the config. When you do that, if the system you're running on
doesn't support it, it will error out rather than switching.
Another thing that I encounter is an error when suspending VM (in
OpenStack environment) when the driver name is vfio.
In such case I am getting the following error from Libvirt:
2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher
File "/usr/lib64/python2.7/site-packages/libvirt.py", line 560, in
attachDeviceFlags
2016-03-28 11:42:59.527 1966 ERROR
oslo_messaging.rpc.dispatcher if ret == -1: raise libvirtError
('virDomainAttachDeviceFlags() failed', dom=self)
2016-03-28 11:42:59.527 1966 ERROR oslo_messaging.rpc.dispatcher
libvirtError: internal error: unable to execute QEMU command
'device_add': Device initialization failed.
I would appreciate for some pointers on what can cause this issue.
Assuming that openstack uses libvirt's virDomainSave API I would
expect suspending a guest to fail if it had an assigned device (since
libvirt implements this by "migrating to disk", and qemu doesn't allow
migration of a guest with an assigned device. But your problem is that
it's trying to *attach* a device, which I wouldn't consider to be a
part of a save or suspend or whatever operation. Is it possible to get
more information about what leads up to this?
[1] https://libvirt.org/formatdomain.html#elementsHostDev
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/31/2016 11:28 AM, Michal Privoznik wrote:
> On 31.03.2016 17:06, Ján Tomko wrote:
>> On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
>>> We use _LAST items in enums to mark the last position in given
>>> enum. Now, if and enum is passed to switch(), compiler checks
>>> that all the values from enum occur in 'case' enumeration.
>>> Including _LAST. But coverity spots it's a dead code. And it
>>> really is. So to resolve this, we tend to put a comment just
>>> above 'case ..._LAST' notifying coverity that we know this is a
>>> dead code but we want to have it that way.
>>>
>>> Signed-off-by: Michal Privoznik
>>> ---
>>> src/qemu/qemu_process.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>
>> ACK.
>>
>> Would it be possible to somehow store a list of false positives
>> for Coverity to ignore in libvirt.git?
>> Like we do with tests/.valgrind.supp.
>>
>> That way we would not need to clutter up the code.
>>
>> Jan
>>
>
> That's what I've been wondering myself too. But frankly, I don't know
> the answer. John?
>
I'd have to dig on this a bit... It is possible to ignore whole classes
of errors using flags to the cov-analyze command; however, that's not
the same as valgrind where you can disable based on stack patterns using
"fun:" prefixes and elipses (...).
The problem with disabling dead_error_begin warnings could be missing
something that truly is a deadcode as opposed to this one where the
reason why the deadcode shows up is because the typed switch() statement
compiler magic requires the VIR_*_LAST: entry instead of going with the
non typed one where the "default:" case would be used.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 30, 2016 at 08:52:38AM +0200, Fabiano Fidêncio wrote:
> Compilation for xdg-app failed due to a buggy SASL headers present on
> the used runtime (org.gnome.Sdk 3.18).
>
> In file included from rpc/virnetsaslcontext.h:24:0,
> from rpc/virnetsaslcontext.c:25:
> /usr/include/sasl/sasl.h:230:38: error: unknown type name 'size_t'
> typedef void *sasl_realloc_t(void *, size_t);
> ^
> /usr/include/sasl/sasl.h:235:5: error: unknown type name 'sasl_realloc_t'
> sasl_realloc_t *,
> ---
> src/rpc/virnetsaslcontext.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
ACK
I have added a reference to commit 1be3dfd and pushed it.
Congrats on your first libvirt patch!
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 31.03.2016 17:06, Ján Tomko wrote:
> On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
>> We use _LAST items in enums to mark the last position in given
>> enum. Now, if and enum is passed to switch(), compiler checks
>> that all the values from enum occur in 'case' enumeration.
>> Including _LAST. But coverity spots it's a dead code. And it
>> really is. So to resolve this, we tend to put a comment just
>> above 'case ..._LAST' notifying coverity that we know this is a
>> dead code but we want to have it that way.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/qemu/qemu_process.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>
> ACK.
>
> Would it be possible to somehow store a list of false positives
> for Coverity to ignore in libvirt.git?
> Like we do with tests/.valgrind.supp.
>
> That way we would not need to clutter up the code.
>
> Jan
>
That's what I've been wondering myself too. But frankly, I don't know
the answer. John?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Instead of relying on substring search, tokenize the input
and process each CPU flag separately. This ensures CPU flag
detection will continue to work correctly even if we start
looking for CPU flags whose name might appear as part of
other CPU flags' names.
The result of processing is stored in a virBitmap, which
means we don't have to parse /proc/cpuinfo in its entirety
for each single CPU flag we want to check.
Moreover, use of the newly-introduced virHostValidateCPUFlag
enumeration ensures we don't go looking for random CPU flags
which might actually be simple typos.
---
Changes in v2:
* use virStringSplitCount() and STRPREFIX() instead of
strtok_r() and strcmp(), as suggested by Peter
tools/virt-host-validate-common.c | 67 ---
tools/virt-host-validate-common.h | 13 +++-
tools/virt-host-validate-qemu.c | 12 +--
3 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index 8ebf73e..3305a32 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -31,7 +31,6 @@
#endif /* HAVE_MNTENT_H */
#include
-#include "virutil.h"
#include "viralloc.h"
#include "virfile.h"
#include "virt-host-validate-common.h"
@@ -39,6 +38,10 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_ENUM_IMPL(virHostValidateCPUFlag, VIR_HOST_VALIDATE_CPU_FLAG_LAST,
+ "vmx",
+ "svm");
+
static bool quiet;
void virHostMsgSetQuiet(bool quietFlag)
@@ -188,29 +191,64 @@ int virHostValidateNamespace(const char *hvname,
}
-bool virHostValidateHasCPUFlag(const char *name)
+virBitmapPtr virHostValidateGetCPUFlags(void)
{
-FILE *fp = fopen("/proc/cpuinfo", "r");
-bool ret = false;
+FILE *fp;
+virBitmapPtr flags;
-if (!fp)
-return false;
+if (!(fp = fopen("/proc/cpuinfo", "r")))
+return NULL;
+
+if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST)))
+return NULL;
do {
char line[1024];
+char *start;
+char **tokens;
+size_t ntokens;
+size_t i;
if (!fgets(line, sizeof(line), fp))
break;
-if (strstr(line, name)) {
-ret = true;
-break;
+/* The line we're interested in is marked either as "flags" or
+ * as "Features" depending on the architecture, so check both
+ * prefixes */
+if (!STRPREFIX(line, "flags") && !STRPREFIX(line, "Features"))
+continue;
+
+/* fgets() includes the trailing newline in the output buffer,
+ * so we need to clean that up ourselves. We can safely access
+ * line[strlen(line) - 1] because the checks above would cause
+ * us to skip empty strings */
+line[strlen(line) - 1] = '\0';
+
+/* Skip to the separator */
+if (!(start = strstr(line, ":")))
+continue;
+
+/* Split the line using " " as a delimiter. The first token
+ * will always be ":", but that's okay */
+if (!(tokens = virStringSplitCount(start, " ", 0, &ntokens)))
+continue;
+
+/* Go through all flags and check whether one of those we
+ * might want to check for later on is present; if that's
+ * the case, set the relevant bit in the bitmap */
+for (i = 0; i < ntokens; i++) {
+int value;
+
+if ((value = virHostValidateCPUFlagTypeFromString(tokens[i])) >= 0)
+ignore_value(virBitmapSetBit(flags, value));
}
+
+virStringFreeListCount(tokens, ntokens);
} while (1);
VIR_FORCE_FCLOSE(fp);
-return ret;
+return flags;
}
@@ -359,15 +397,20 @@ int virHostValidateCGroupController(const char *hvname,
int virHostValidateIOMMU(const char *hvname,
virHostValidateLevel level)
{
+virBitmapPtr flags;
struct stat sb;
const char *bootarg = NULL;
bool isAMD = false, isIntel = false;
-if (virHostValidateHasCPUFlag("vmx"))
+flags = virHostValidateGetCPUFlags();
+
+if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
isIntel = true;
-else if (virHostValidateHasCPUFlag("svm"))
+else if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_SVM))
isAMD = true;
+virBitmapFree(flags);
+
virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
if (isIntel) {
diff --git a/tools/virt-host-validate-common.h
b/tools/virt-host-validate-common.h
index d4c4759..26e006b 100644
--- a/tools/virt-host-validate-common.h
+++ b/tools/virt-host-validate-common.h
@@ -23,6 +23,8 @@
# define __VIRT_HOST_VALIDATE_COMMON_H__
# include "internal.h"
+# include "virutil.h"
+# include "virbitmap.h"
typedef enum {
VIR_HOST_VALIDATE_FAIL,
@@ -32,6 +34,15 @@ typedef enum {
VIR_HOST_VALIDATE_LAST,
} virHostValidateLevel;
+typedef
On Wed, Mar 30, 2016 at 04:36:03PM -0600, Jim Fehlig wrote:
> Chunyan sent a nice cleanup patch for libxlDomainDetachNetDevice
>
> https://www.redhat.com/archives/libvir-list/2016-March/msg00926.html
>
> which I incorrectly modified before pushing as commit b5534e53. My
> modification caused network devices of type hostdev to no longer
> be removed. This patch changes b5534e53 to resemble Chunyan's
> original, correct patch.
>
> Signed-off-by: Jim Fehlig
> ---
> src/libxl/libxl_driver.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
ACK
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 30, 2016 at 04:36:02PM -0600, Jim Fehlig wrote:
> Chunyan sent a correct patch to fix a resource leak on error in
> libxlDomainAttachNetDevice
>
> https://www.redhat.com/archives/libvir-list/2016-March/msg00924.html
>
> I made what was thought to be an improvement and pushed the patch as
> commit e6336442. As it turns out, my change broke adding net devices
> that are actually hostdevs to the list of nets in virDomainDef. This
> patch changes e6336442 to resemble Chunyan's original, correct
> patch.
>
> Signed-off-by: Jim Fehlig
> ---
> src/libxl/libxl_driver.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
ACK
Regards,
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Le 29/03/2016 16:48, Jean-Marc Liger a écrit :
Le 11/03/2016 15:18, Jean-Marc LIGER a écrit :
There is a trigger already defined issue when you try to rebuild
libvirt >= 1.3.0 for el6 with copr and most probably koji.
Regards,
Jean-Marc
I've modified my patch after the nss update, someone could revue this
patch before the next release ?
Regards,
Jean-Marc
diff -uri a/libvirt.spec.in b/libvirt.spec.in
--- a/libvirt.spec.in2016-03-29 16:40:37.985927719 +0200
+++ b/libvirt.spec.in2016-03-29 16:28:01.808864888 +0200
@@ -1783,16 +1783,6 @@
fi
%endif
-%if %{with_systemd}
-%else
-%triggerpostun daemon -- libvirt-daemon < 1.2.1
-if [ "$1" -ge "1" ]; then
-/sbin/service virtlockd reload > /dev/null 2>&1 || :
-/sbin/service virtlogd reload > /dev/null 2>&1 || :
-/sbin/service libvirtd condrestart > /dev/null 2>&1
-fi
-%endif
-
# In upgrade scenario we must explicitly enable virtlockd/virtlogd
# sockets, if libvirtd is already enabled and start them if
# libvirtd is running, otherwise you'll get failures to start
@@ -1805,6 +1795,12 @@
/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 &&
/bin/systemctl start virtlogd.socket || :
%else
+%triggerpostun daemon -- libvirt-daemon < 1.2.1
+if [ "$1" -ge "1" ]; then
+/sbin/service virtlockd reload > /dev/null 2>&1 || :
+/sbin/service virtlogd reload > /dev/null 2>&1 || :
+/sbin/service libvirtd condrestart > /dev/null 2>&1
+fi
/sbin/chkconfig libvirtd 1>/dev/null 2>&1 &&
/sbin/chkconfig virtlogd on || :
/sbin/service libvirtd status 1>/dev/null 2>&1 &&
Maybe this is the wrong way to fix this problem. But it exists for non
systemd builds, where directives %triggerpostun daemon -- libvirt-daemon
< 1.2.1 and %triggerpostun daemon -- libvirt-daemon < 1.3.0 are examined
sequentialy, which is causing a trigger issue.
https://copr.fedorainfracloud.org/coprs/jmliger/tests-upstream/build/172145/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
> We use _LAST items in enums to mark the last position in given
> enum. Now, if and enum is passed to switch(), compiler checks
> that all the values from enum occur in 'case' enumeration.
> Including _LAST. But coverity spots it's a dead code. And it
> really is. So to resolve this, we tend to put a comment just
> above 'case ..._LAST' notifying coverity that we know this is a
> dead code but we want to have it that way.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_process.c | 2 ++
> 1 file changed, 2 insertions(+)
>
ACK.
Would it be possible to somehow store a list of false positives
for Coverity to ignore in libvirt.git?
Like we do with tests/.valgrind.supp.
That way we would not need to clutter up the code.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 04:48:23PM +0200, Michal Privoznik wrote:
> Do I really need to explain why?
Do you really need to ask?
> Well, if read() is interrupted int the middle of reading, we will
> never read the rest (even though it's highly unlikely as we are
> reading just 8 bytes).
>
> Signed-off-by: Michal Privoznik
> ---
> src/util/virperf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
ACK
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
We use _LAST items in enums to mark the last position in given
enum. Now, if and enum is passed to switch(), compiler checks
that all the values from enum occur in 'case' enumeration.
Including _LAST. But coverity spots it's a dead code. And it
really is. So to resolve this, we tend to put a comment just
above 'case ..._LAST' notifying coverity that we know this is a
dead code but we want to have it that way.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_process.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 07b9df2..e58bf16 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3953,6 +3953,8 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
virDomainHypervTypeToString(i));
goto cleanup;
break;
+
+/* coverity[dead_error_begin] */
case VIR_DOMAIN_HYPERV_LAST:
break;
}
--
2.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Do I really need to explain why?
Well, if read() is interrupted int the middle of reading, we will
never read the rest (even though it's highly unlikely as we are
reading just 8 bytes).
Signed-off-by: Michal Privoznik
---
src/util/virperf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 9dc4e25..359a9c3 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -247,7 +247,7 @@ virPerfReadEvent(virPerfPtr perf,
if (event == NULL || !event->enabled)
return -1;
-if (read(event->fd, value, sizeof(uint64_t)) < 0) {
+if (saferead(event->fd, value, sizeof(uint64_t)) < 0) {
virReportSystemError(errno, "%s",
_("Unable to read cache data"));
return -1;
--
2.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2016-03-31 at 07:25 -0400, John Ferlan wrote:
> On 03/30/2016 10:19 AM, Andrea Bolognani wrote:
> >
> > Basically, the navigation menu is *way* too big, and takes
> > the focus away from the actual content. Plus it's very easy
> > for longish words to overflow it (see "Authentication" in
> > the screenshots below).
> >
> > The headers are probably bigger than needed as well, so
> > I've scaled them down. Using Wikipedia for comparison,
> > you can see the new size is more comparable to what they
> > use.
> >
> > Some other minor cleanups have been included as well.
> >
> > Before[1] and after[2] screenshots for your convenience.
> >
> > Cheers.
> >
> >
> > [1] http://i.imgur.com/MZchAWD.png
> > [2] http://i.imgur.com/BXepLTh.png
> >
> > Andrea Bolognani (7):
> > docs: Adjust vertical whitespace in CSS
> > docs: Remove empty CSS rule
> > docs: Don't use in headers
> > docs: Make menu entries smaller
> > docs: Don't use bold text for menu entries
> > docs: Use bold text for all headers
> > docs: Make most headers a bit smaller
> >
> > docs/apps.html.in | 2 +-
> > docs/generic.css | 15 +--
> > docs/libvirt.css | 30 ++
> > 3 files changed, 16 insertions(+), 31 deletions(-)
>
> Lot's of style churn here lately... Ironically the view I get from
> libvirt.org has always had slightly bolder and larger text as opposed
> the view I get when viewing changes in a local branch.
That... Shouldn't happen.
I wonder if you increased the font size while viewing
libvirt.org at some point, and your browser is remembering that
as a per-website preference?
> Personally, I
> like the larger, bolder text since it's easier to read than some font
> style that's meant for a mobile phone.
The idea behind Cole's initial changes, if I'm not mistaken,
was to make the website more readable by using sites such as
Wikipedia as reference. So better spacing etc. Big font sizes
are good, but the text shouldn't be comically large.
> As long as we don't start adding
> graphics and flash, we're good ;-)!
Don't even think about it!
> Anyway just a note that patch 3 removes the h1 font-weight bold, but
> patch 6 claims all headers except h1 were already bold... Not a big deal
> due to the end result, but I assume they should be together.
All headers except for were already bold: what patch 3
does is removing the only instance in which bold text was
used as part of a (non bold) header, and the CSS rules
needed for that specific case.
Patch 6 then goes ahead and makes bold like all other
headers.
> My only other comment would be regarding elements - I would prefer
> to see them use a slightly bolder text so that they stand out from the
> rest of the text. If I read the tea leaves correctly that would be an
> adjustment to the to have "font-weight: bold;".
I didn't notice that, but I'll look into changing it next.
> Other than that you have an ACK from me... and it should be safe for
> freeze...
I've pushed the series. Thanks!
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 31.03.2016 08:16, Michal Privoznik wrote:
> On 30.03.2016 04:13, Qiaowei Ren wrote:
>> This patch adds the python binding for virDomainSetPerfEvents and
>> virDomainSetPerfEvents API.
>>
>> Signed-off-by: Qiaowei Ren
>> ---
>> generator.py | 2 ++
>> libvirt-override-api.xml | 11 ++
>> libvirt-override.c | 93
>>
>> 3 files changed, 106 insertions(+)
>>
I had to do couple of more tweaks than originally intended. But this is
now pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/30/2016 11:14 AM, Martin Kletzander wrote:
> Since commit 714080791778e3dfbd484ccb3953bffd820b8ba9, qemu agent
> channel cannot be plugged in because we won't generate its path
> automatically. Let's not only fix that, but also add tests for it so
> next time it's checked for.
>
Save some electrons, shorten the commit id
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1322210
>
> Signed-off-by: Martin Kletzander
> ---
> src/qemu/qemu_hotplug.c| 3 ++
> tests/qemuhotplugtest.c| 15 ++
> .../qemuhotplug-hotplug-base+qemu-agent-detach.xml | 58
> ++
> .../qemuhotplug-hotplug-base+qemu-agent.xml| 58
> ++
> .../qemuhotplug-qemu-agent-detach.xml | 5 ++
> .../qemuhotplugtestdata/qemuhotplug-qemu-agent.xml | 5 ++
> 6 files changed, 144 insertions(+)
> create mode 100644
> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent-detach.xml
> create mode 100644
> tests/qemuhotplugtestdata/qemuhotplug-hotplug-base+qemu-agent.xml
> create mode 100644
> tests/qemuhotplugtestdata/qemuhotplug-qemu-agent-detach.xml
> create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-qemu-agent.xml
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index e82dbf5448dc..b7741e15d445 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1565,6 +1565,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> goto cleanup;
> }
>
> +if (qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
> +goto cleanup;
> +
> if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
> goto cleanup;
>
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 2b0de94fb4a6..384b7b9592b9 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -98,6 +98,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
>
> (*vm)->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID;
>
> +if (qemuDomainSetPrivatePaths(&priv->libDir,
> + &priv->channelTargetDir,
> + "/var/lib/libvirt",
> + "/var/lib/libvirt/qemu/channel/target",
> + (*vm)->def->name,
> + (*vm)->def->id) < 0)
I believe this overwrites qemuTestDriverInit - since it's a test I'm not
concerned about the memory leak, just the processing consistency since
you're not really starting a guest, why change the paths?
While it's fresh in my mind (still) using /tmp/* in *DriverInit when I
was generating patches the domain master secret key file caused problems
if I actually tried to check for the existence, especially since
qemuProcessPrepareHost is where the qemuProcessMakeDir calls were made
to create the directory structure. Perhaps if the tests driver created
"tmp/*" paths rather than "/tmp/*" paths that'd work, but is more or
less unrelated.
I'll wait for your thoughts on this before an official ACK -
John
> +goto cleanup;
> +
> ret = 0;
> cleanup:
> return ret;
> @@ -495,6 +503,13 @@ mymain(void)
> "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK,
> "human-monitor-command", HMP(""));
>
> +DO_TEST_ATTACH_LIVE("hotplug-base", "qemu-agent", false, true,
> +"chardev-add", QMP_OK,
> +"device_add", QMP_OK);
> +DO_TEST_DETACH("hotplug-base", "qemu-agent-detach", false, false,
> + "device_del", QMP_OK,
> + "chardev-remove", QMP_OK);
> +
> qemuTestDriverFree(&driver);
> return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
> }
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/30/2016 10:19 AM, Andrea Bolognani wrote:
> Basically, the navigation menu is *way* too big, and takes
> the focus away from the actual content. Plus it's very easy
> for longish words to overflow it (see "Authentication" in
> the screenshots below).
>
> The headers are probably bigger than needed as well, so
> I've scaled them down. Using Wikipedia for comparison,
> you can see the new size is more comparable to what they
> use.
>
> Some other minor cleanups have been included as well.
>
> Before[1] and after[2] screenshots for your convenience.
>
> Cheers.
>
>
> [1] http://i.imgur.com/MZchAWD.png
> [2] http://i.imgur.com/BXepLTh.png
>
> Andrea Bolognani (7):
> docs: Adjust vertical whitespace in CSS
> docs: Remove empty CSS rule
> docs: Don't use in headers
> docs: Make menu entries smaller
> docs: Don't use bold text for menu entries
> docs: Use bold text for all headers
> docs: Make most headers a bit smaller
>
> docs/apps.html.in | 2 +-
> docs/generic.css | 15 +--
> docs/libvirt.css | 30 ++
> 3 files changed, 16 insertions(+), 31 deletions(-)
>
Lot's of style churn here lately... Ironically the view I get from
libvirt.org has always had slightly bolder and larger text as opposed
the view I get when viewing changes in a local branch. Personally, I
like the larger, bolder text since it's easier to read than some font
style that's meant for a mobile phone. As long as we don't start adding
graphics and flash, we're good ;-)!
Anyway just a note that patch 3 removes the h1 font-weight bold, but
patch 6 claims all headers except h1 were already bold... Not a big deal
due to the end result, but I assume they should be together.
My only other comment would be regarding elements - I would prefer
to see them use a slightly bolder text so that they stand out from the
rest of the text. If I read the tea leaves correctly that would be an
adjustment to the to have "font-weight: bold;".
Other than that you have an ACK from me... and it should be safe for
freeze...
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2016-03-31 11:37 GMT+03:00 Vasiliy Tolstov :
>
>
> As i understand libvirt try to determine client alive and fails, my
> code properly close libvirt connection, but may be it not freed? Or
> something else broken with my code?
>
> i'm use libvirt-python from 1.2.1 to 1.2.9 and libvirt from 1.3.0 to 1.3.3
Please forget, this is errors because some nagios plugin check tcp
socket of libvirt and libvirtd thinks that this is proper client
connecting.
--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2016-03-23 20:46 GMT+03:00 Laine Stump :
> Since there is no documentation included with the patch, and the wrong RNG
> file has been modified, I'm not clear on exactly why a libvirt virtual
> network would use a peer address.
>
> Normally libvirt networks are made by creating a bridge device, adding in
> some iptables rules, and running an instance of dnsmasq to service dhcp and
> dns requests made by guests who have tap devices connected to that network.
> But if I understand correctly, your patches are intended to allow setting
> the local and peer address for guest-connected tap devices that aren't
> attached to a bridge on the host side, but instead rely on the host's IP
> stack to route appropriate traffic through the tap device. If so, then why
> is a libvirt network involved at all? Why/how could a bridge device be used
> for a point-to-point link? If this isn't just a misunderstanding of which
> parts of libvirt code affect what, then some examples (and patches to
> formatdomain.html.in/formatnetwork.html.in) would be very useful to help me
> understand.
I'm send new patch with some docs in formatdomain.
Sometimes bridges not allowed or not needed. We use plain tap devices
on host side and bird routing daemon to route traffic to/from tap
devices.
--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Allow to use ip address and routes elements inside network ethernet.
Also allow to configure point-to-point address for host side device.
Signed-off-by: Vasiliy Tolstov
---
docs/formatdomain.html.in| 12 -
docs/schemas/network.rng | 3 +++
include/libvirt/libvirt-domain.h | 1 +
src/conf/domain_conf.c | 14 ++-
src/conf/domain_conf.h | 1 +
src/conf/network_conf.c | 26 ++-
src/conf/network_conf.h | 1 +
src/lxc/lxc_container.c | 2 +-
src/network/bridge_driver.c | 2 +-
src/qemu/qemu_interface.c| 39 +
src/util/virnetdev.c | 54
src/util/virnetdev.h | 1 +
12 files changed, 135 insertions(+), 21 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 71ffe750452e..1203e96a8286 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4717,6 +4717,7 @@ qemu-kvm -net nic,model=? /dev/null
+
@@ -4749,7 +4750,16 @@ qemu-kvm -net nic,model=? /dev/null
to define the network routes to use for the network device. The attributes
of this element are described in the documentation for the
route
element in network
definitions.
-This is only used by the LXC driver.
+This is used by the LXC driver and Since 1.3.3
by the QEMU
+driver.
+
+
+
+Since 1.3.3 ip elements can hold peer attribute
to assign
+point-to-point address for the network device. The attributes of this
element
+are described in the documentation for the ip element in
+network definitions.
+This is only used by the LXC and QEMU driver.
vhost-user interface
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 4edb6eb31ad0..47b4b80fc0c3 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -315,6 +315,9 @@
+
+
+
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7e06796c3c73..437e87fac01c 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3937,6 +3937,7 @@ typedef virDomainIPAddress *virDomainIPAddressPtr;
struct _virDomainInterfaceIPAddress {
int type;/* virIPAddrType */
char *addr; /* IP address */
+char *peer; /* IP peer */
unsigned int prefix; /* IP address prefix */
};
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d5d9ff702f42..34855233ad15 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5725,7 +5725,7 @@ virDomainNetIpParseXML(xmlNodePtr node)
unsigned int prefixValue = 0;
char *familyStr = NULL;
int family = AF_UNSPEC;
-char *address = NULL;
+char *address = NULL, *peer = NULL;
if (!(prefixStr = virXMLPropString(node, "prefix")) ||
(virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) {
@@ -5739,6 +5739,9 @@ virDomainNetIpParseXML(xmlNodePtr node)
goto cleanup;
}
+if ((peer = virXMLPropString(node, "peer")) == NULL)
+VIR_DEBUG("Peer is empty");
+
familyStr = virXMLPropString(node, "family");
if (familyStr && STREQ(familyStr, "ipv4"))
family = AF_INET;
@@ -5756,6 +5759,14 @@ virDomainNetIpParseXML(xmlNodePtr node)
address);
goto cleanup;
}
+
+if ((peer != NULL) && (virSocketAddrParse(&ip->peer, peer, family) < 0)) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("Failed to parse IP address: '%s'"),
+ peer);
+goto cleanup;
+}
+
ip->prefix = prefixValue;
ret = ip;
@@ -5765,6 +5776,7 @@ virDomainNetIpParseXML(xmlNodePtr node)
VIR_FREE(prefixStr);
VIR_FREE(familyStr);
VIR_FREE(address);
+VIR_FREE(peer);
VIR_FREE(ip);
return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 83bdd67dec45..040882ec8460 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -513,6 +513,7 @@ typedef struct _virDomainNetIpDef virDomainNetIpDef;
typedef virDomainNetIpDef *virDomainNetIpDefPtr;
struct _virDomainNetIpDef {
virSocketAddr address; /* ipv4 or ipv6 address */
+virSocketAddr peer;/* ipv4 or ipv6 address of peer */
unsigned int prefix; /* number of 1 bits in the net mask */
};
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.
This is next part of network type ethernet patches, that adds ability to assign
ip address and routes to tap devices for qemu driver like done before for lxc.
Vasiliy Tolstov (1):
allow ip and route elements for netdev ethernet
docs/formatdomain.html.in| 12 -
docs/schemas/network.rng | 3 +++
include/libvirt/libvirt-domain.h | 1 +
src/conf/domain_conf.c | 14 ++-
src/conf/domain_conf.h | 1 +
src/conf/network_conf.c | 26 ++-
src/conf/network_conf.h | 1 +
src/lxc/lxc_container.c | 2 +-
src/network/bridge_driver.c | 2 +-
src/qemu/qemu_interface.c| 39 +
src/util/virnetdev.c | 54
src/util/virnetdev.h | 1 +
12 files changed, 135 insertions(+), 21 deletions(-)
--
2.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2016-03-30 at 13:19 -0400, John Ferlan wrote:
> On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> >
> > The struct contains a single boolean field which can be
> > applied to domain capabilities that do not represent device
> > availability.
> >
> > Instead of trying to come up with a more generic name just
> > get rid of the struct altogether.
> > ---
> > src/conf/domain_capabilities.c | 6 +++---
> > src/conf/domain_capabilities.h | 14 --
> > src/qemu/qemu_capabilities.c | 8
> > tests/domaincapstest.c | 8
> > 4 files changed, 15 insertions(+), 21 deletions(-)
>
> The construct was added as part of commit id '614581f32' - not sure if
> Michal felt more bool's were going to be added to virDomainCapsDevice.
The problem is that, as you noted, patch 4 adds a
virDomainCapsFeatureGIC structure - if I were to follow the
pattern established with eg. virDomainCapsDeviceDisk, I would
have to introduce something like
struct _virDomainCapsFeature {
bool supported;
};
and then use it as the first member of virDomainCapsFeatureGIC;
however, that would clash with the virDomainCapsFeature that's
already being defined in domain_conf.h.
Moreover, the existing FORMAT_PROLOGUE() macro would not be
usable for virDomainCapsFeatureGIC, because it checks for
item->device.supported and we would probably use something
like item->feature.supported instead.
Last but not least, the current usage is questionable: neither
virDomainCapsOS nor virDomainCapsLoader end up being inside the
element, yet both include virDomainCapsDevice as
their first member...
> I see this affects patch 4 - I think it would be a good idea to see if
> Michal had "other designs" in mind before changing this. That could be
> separate too...
CCing Michal so he can voice his opinion on the matter.
Cheers.
PS: Don't worry, I have no idea what I was trying to say with
the first paragraph of the commit message either :)
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Michal Privoznik
---
src/libvirt-domain.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3e144b6..4f473c9 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9695,11 +9695,13 @@ virDomainOpenChannel(virDomainPtr dom,
* @domain: a domain object
* @params: where to store perf events setting
* @nparams: number of items in @params
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainModificationImpact
*
- * Get all perf events setting. Possible fields returned in @params are
- * defined by VIR_DOMAIN_PERF_* macros and new fields will likely be
- * introduced in the future.
+ * Get all Linux perf events setting. Possible fields returned in
+ * @params are defined by VIR_PERF_EVENT_* macros and new fields
+ * will likely be introduced in the future.
+ *
+ * Linux perf events are performance analyzing tool in Linux.
*
* Returns -1 in case of failure, 0 in case of success.
*/
@@ -9743,9 +9745,13 @@ int virDomainGetPerfEvents(virDomainPtr domain,
* @params: pointer to perf events parameter object
* @nparams: number of perf event parameters (this value can be the same
* less than the number of parameters supported)
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virDomainModificationImpact
*
- * Enable or disable the particular list of perf events you care about.
+ * Enable or disable the particular list of Linux perf events you
+ * care about. The @params argument should contain any subset of
+ * VIR_PERF_EVENT_ macros.
+ *
+ * Linux perf events are performance analyzing tool in Linux.
*
* Returns -1 in case of error, 0 in case of success.
*/
--
2.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 23, 2016 at 15:34:07 +0300, Nikolay Shirokovskiy wrote:
> >> +typedef struct _qemuMonitorMigrationParameters
> >> qemuMonitorMigrationParameters;
> >> +typedef qemuMonitorMigrationParameters *qemuMonitorMigrationParametersPtr;
> >> +struct _qemuMonitorMigrationParameters {
> >> +unsigned int level_set : 1;
> >> +unsigned int threads_set : 1;
> >> +unsigned int dthreads_set : 1;
> >> +
> >> +int level;
> >> +int threads;
> >> +int dthreads;
> >> +};
> >
> > Actually, I think the names should correspond to the names used by QEMU
> > to avoid some confusion.
>
> Ouch, this reveals some more misdesigned stuff. Look, I put
> qemuMonitorMigrationParameters
> into qemuMigrationCompression which is a kind of reverse aggregation. I see
> two
> options to make things the proper way here.
>
> 1. Rename qemuMonitorMigrationParameters, qemuMonitorSetMigrationParameters
> etc
> to add compression to the naming somehow. If actual qemu command will one
> day be extended to support parameters from different category - well we
> just add another json monitor command that will get/set the new subset
> of parametes.
>
> 2. Rework code so that we will aggregate all migration parameters into
> qemuMonitorSetMigrationParameters value and call actual qemu command
> only once. This way either we pack all compression values
> into one structure and have code that fills migration parameters value
> appropriately or we stop keeping compression related parameters
> in qemuMigrationCompression. I don't like any of these.
>
> So i prefer the first option.
Originally, I inclined to some form of option 2, but now I agree option
1 would be better. At least until we have other migration parameters,
but we can rethink the design if needed then.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I'm use libvirt-python binding to export some metrics to ganglia from libvirt.
python code http://paste.debian.net/422944/
when this script is running i get many erros in libvirtd.log like this:
error : virNetSocketReadWire:1613 : End of file while reading data:
Input/output error
При этом открывается сокет:
virNetSocketNew:277 : RPC_SOCKET_NEW: sock=0x7fced8036ae0 fd=27
errfd=-1 pid=0 localAddr=::1;16509, remoteAddr=::1;39962
As i understand libvirt try to determine client alive and fails, my
code properly close libvirt connection, but may be it not freed? Or
something else broken with my code?
i'm use libvirt-python from 1.2.1 to 1.2.9 and libvirt from 1.3.0 to 1.3.3
--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 30, 2016 at 18:35:04 +0200, Andrea Bolognani wrote:
> On Wed, 2016-03-30 at 09:28 +0200, Andrea Bolognani wrote:
> > On Wed, 2016-03-30 at 08:36 +0200, Peter Krempa wrote:
> > >
> > > Since you are already using libvirt's utils ...
> > [...]
> > >
> > > Why not virStringSplit ...
> > [...]
> > >
> > > And STREQ?
> >
> > No reason at all, really :)
>
> On second thought, after having actually tried to implement
> your suggestions...
>
> virStringSplit() only supports a single delimiter, while
> strtok_r() supports an arbitrary number - I'm using three in
> my case.
>
> So AFAICS the options are
>
> - call virStringSplit() multiple times, hoping to take
> all possible combinations into account
Humm, no that wouldn't make much sense.
> - enhance virStringSplit() to support multiple delimiters
virStringSplit actually uses a string as delimiter for some reason so
that option would basically result in adding a new function.
>
> - keep using strtok_r()
Okay, it makes sense here :)
>
> I'm quite partial to the last option :P
> What about you?
Let me re-visit the patch then.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 31.03.2016 09:53, Jiri Denemark wrote:
> On Thu, Mar 24, 2016 at 11:08:31 +0300, Nikolay Shirokovskiy wrote:
>>> The backward compatibility code will be limited to Parse/Dump parameters
>>> and the rest of the code will just see a nice struct of compression
>>> parameters without having to worry about how it was called. And dealing
>>> with it in *Dump is easy, just set don't output any parameters if only
>>> XBZRLE method was selected and no compression parameter is set.
>>
>> Then I should never pass NULL value of compression pointer anywhere.
>> That is I should fix all the places where this is done in current version.
>> Below is the complete list of functions where NULL is passed:
>>
>> qemuDomainMigratePrepare2
>> qemuDomainMigratePrepare3
>> qemuDomainMigratePerform
>> qemuDomainMigratePerform3
>
> None of these accepts any compression parameters (except for the flag
> itself) so you could use a simple struct with only xbzrle set (or not
> depending on the flag) and place it on the stack, but the code would
> look cleaner if you just called the parse/free function too even though
> they wouldn't do much without any typed parameters passed in.
>
>> qemuMigrationPrepareTunnel
>> doPeer2PeerMigrate2
>> qemuMigrationPerformJob
>>
>> I need to add code to parse/free in every of this places. This is
>> my concern. Except this I totally agree that trying to put
>> backward compatibility issues in one place like parse/dump
>> functions is good. But on this way I still need to touch
>> a lot of places in less trivial way (in comparsion to
>> don't mixing flags / compression parameters).
>
> Right, but you touch various entry points and the internal code that
> actually does something interesting doesn't need to care about the
> origin of compression parameters. That said, I didn't really try
> changing the code to see how it looks like but having a single structure
> describing compression methods and parameters no matter how they were
> specified by a user seems cleaner to me.
>
> Jirka
>
Thanx for clarification!
Nikolay
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:28:55 +0200, Michal Privoznik wrote:
> I really like to see these in before release and thus APIs get
> written in the stone.
>
> One note though regarding the last patch: while writing it I've
> noticed couple of other getter APIs don't grab any job at all. I
> think they should grab _QUERY job though. So maybe those will
> need some fixing too.
I've more-or-less ACKed the series, but it would be great if you could
improve the documentation for the new APIs since it really doesn't tell
anybody what's happening there.
Additionally the docs for 'virDomainGetPerfEvents' states that @params
is filled by 'VIR_DOMAIN_PERF_*' macros, but in reality the macro has a
different prefix: VIR_PERF_PARAM_CMT
Thanks for cleaning up the mess though.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:28:57 +0200, Michal Privoznik wrote:
> I've noticed that these APIs are missing @flags argument. Even
> though we don't have a use for them, it's our policy that every
> new API must have @flags.
>
> Signed-off-by: Michal Privoznik
> ---
> daemon/remote.c | 2 +-
> include/libvirt/libvirt-domain.h | 6 --
> src/driver-hypervisor.h | 6 --
> src/libvirt-domain.c | 22 ++
> src/qemu/qemu_driver.c | 11 ---
> src/remote/remote_driver.c | 4 +++-
> src/remote/remote_protocol.x | 2 ++
> tools/virsh-domain.c | 5 +++--
> 8 files changed, 39 insertions(+), 19 deletions(-)
>
> struct remote_domain_get_perf_events_ret {
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index cda442d..a105268 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -8610,6 +8610,7 @@ cmdPerf(vshControl *ctl, const vshCmd *cmd)
> virTypedParameterPtr params = NULL;
> bool ret = false;
> const char *enable = NULL, *disable = NULL;
> +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
There is no such flag option currently, you should pass 0. (I know that
VIR_DOMAIN_AFFECT_CURRENT is actually equal to 0, but in the context of
this patch you are stating that there are no supported flags.)
>
> if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> return false;
ACK, disregard my above statement. Fixing the 'unpleasant' code is more
important at this point.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:29:01 +0200, Michal Privoznik wrote:
> Even though we have the machine locked throughout whole APIs we
> are querying/modifying domain internal state. We should grab a
> job whilst doing that.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_driver.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
ACK, but you will get merge conflicts after fixing 5/6
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:28:59 +0200, Michal Privoznik wrote:
> Everywhere else we use a comma separated list. There's no good
> reason to make 'perf' command an exception. Currently, it accepts
> string list separated by '|'.
>
> Signed-off-by: Michal Privoznik
> ---
> tools/virsh-domain.c | 2 +-
> tools/virsh.pod | 17 +
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index a09cdec..2dbb890 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -8575,7 +8575,7 @@ virshParseEventStr(vshControl *ctl,
> size_t i, ntok;
> int ret = -1;
>
> -if (!(tok = virStringSplitCount(event, "|", 0, &ntok)))
> +if (!(tok = virStringSplitCount(event, ",", 0, &ntok)))
> return -1;
>
> if (ntok > VIR_PERF_EVENT_LAST) {
While at it, you could also remove this condition. It doesn't make sense
with typed param apis and hinders forward compatibility.
ACK with or without the condition removed.
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:28:56 +0200, Michal Privoznik wrote:
> There are few lines off the indentation.
>
> Signed-off-by: Michal Privoznik
> ---
> src/remote/remote_driver.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
ACK
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 31, 2016 at 07:28:58 +0200, Michal Privoznik wrote:
> We have a macro that does exactly what is done via full
> enumeration.
>
> Signed-off-by: Michal Privoznik
> ---
> tools/virsh-domain.c | 6 +-
> 1 file changed, 1 insertion(+), 5 deletions(-)
I don't prefer the macro, but ACK for consistency
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
