[nbdkit PATCH] nbd: Fix race during close
by Eric Blake
ThreadSanitizer [1] pointed out that in the nbd plugin, nbd_close() can
attempt close() in the main thread while the worker thread is still
attempting to start a read(). Normally, if the read() loses the race,
it will get a harmless EBADF that exits the worker thread (which is what
we want, as we are closing the connection anyway); but if another
connection happens to start in that window, we could end up read()ing
from the fd opened by the new connection, with disastrous results on the
second connection.
[1] ./configure CXFLAGS=-fsanitize=thread LDFLAGS=-fsanitize=thread
Commits c70616f8 and 430f8141 tried to clean up deadlock during
shutdown, but missed that without some sort of locking, a
close-before-read was still possible. Swap lines so that pthread_join()
now serves as the locking to ensure close is not attempted while
another thread may be about to use the fd.
Thanks: Richard W.M. Jones
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
It took me a while to decipher how ThreadSanitizer actually tests
this race, which gets reported as a Write guarded by a mutex [caused
by close()] racing with an earlier Read [caused by read()]. I
finally realized that linking with libtsan installs wrappers around
the syscalls for read(), close(), etc. where the wrappers create an
underlying mutex and read/write operations on sentinel memory, so that
it can then reuse its memory race analysis it has for more typical
data races. The wrappers thus cause odd-looking reports for fd races
(the report ends up claiming that Thread 1 performing close() lost a
race to Thread 2 performing read() - even though the ACTUAL data race
is only a bug when Thread 2 loses the race and read()s on an fd
close()d by Thread 1 and possibly reused by Thread 3 in the meantime).
plugins/nbd/nbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index b9a4523..9130642 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -575,9 +575,9 @@ nbd_close (void *handle)
nbd_request_raw (h, 0, NBD_CMD_DISC, 0, 0, 0, NULL);
shutdown (h->fd, SHUT_WR);
}
- close (h->fd);
if ((errno = pthread_join (h->reader, NULL)))
nbdkit_debug ("failed to join reader thread: %m");
+ close (h->fd);
pthread_mutex_destroy (&h->write_lock);
pthread_mutex_destroy (&h->trans_lock);
free (h);
--
2.17.2
5 years, 6 months
[PATCH v2 0/3] Install QEMU-GA from oVirt guest tools ISO on Linux
by Tomáš Golembiovský
changes in v2:
- moved copy_drivers above copy_files
- renamed copy_files to copy_from_virtio_win
- renamed install to install_local
- use rpm instead of yum
This installs packages with QEMU Guest Agent when converting Linux machine. The
packages should be available on guest tools ISO. The patches work "as-is" but
probably deserve some more attention:
- it is "abusing" Winows_virtio code but renaming/refactoring everything to
remove "windows" from the name and use "guest tools" seems like a lot of
unnecesary work
- support for Debian is missing
I don't know how to install the package only when all it's dependencies are
already installed. dpkg cannot be used to check that (simulate the install).
And attempting to install the package will leave it half-installed (dpkg
cannot roll-back).
Tomáš Golembiovský (3):
v2v: refactor copy_drivers() in Windows_virtio
v2v: linux: install packages
v2v: linux: install QEMU-GA (RHBZ#1619665)
v2v/convert_linux.ml | 2 ++
v2v/linux.ml | 14 ++++++++
v2v/linux.mli | 3 ++
v2v/windows_virtio.ml | 78 +++++++++++++++++++++++++++++++++---------
v2v/windows_virtio.mli | 4 +++
5 files changed, 84 insertions(+), 17 deletions(-)
--
2.19.0
5 years, 6 months
[PATCH 0/3] Install QEMU-GA from oVirt guest tools ISO on Linux
by Tomáš Golembiovský
This installs packages with QEMU Guest Agent when converting Linux machine. The
packages should be available on guest tools ISO. The patches work "as-is" but
probably deserve some more attention:
- it is "abusing" Winows_virtio code but renaming/refactoring everything to
remove "windows" from the name and use "guest tools" seems like a lot of
unnecesary work
- support for Debian is missing
I don't know how to install the package only when all it's dependencies are
already installed. dpkg cannot be used to check that (simulate the install).
And attempting to install the package will leave it half-installed (dpkg
cannot roll-back).
Tomáš Golembiovský (3):
v2v: refactor copy_drivers() in Windows_virtio
v2v: linux: install packages
v2v: linux: install QEMU-GA
v2v/convert_linux.ml | 2 ++
v2v/linux.ml | 19 ++++++++++
v2v/linux.mli | 3 ++
v2v/windows_virtio.ml | 82 +++++++++++++++++++++++++++++++-----------
v2v/windows_virtio.mli | 5 +++
5 files changed, 91 insertions(+), 20 deletions(-)
--
2.19.0
5 years, 6 months
[PATCH] p2v: use newer GTK APIs if possible
by Pino Toscano
Make use of APIs available in early GTK+ 3 versions, adding
compatibility functions/macros for older GTK+ versions.
There is no behaviour change, and it helps in porting to even newer GTK+
versions.
---
p2v/gui-gtk3-compat.h | 16 ++++++++++++++++
p2v/gui.c | 13 +++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/p2v/gui-gtk3-compat.h b/p2v/gui-gtk3-compat.h
index 8f32eadbf..04923535e 100644
--- a/p2v/gui-gtk3-compat.h
+++ b/p2v/gui-gtk3-compat.h
@@ -17,6 +17,18 @@
*/
/* Backwards compatibility for some deprecated functions in Gtk 3. */
+#if !GTK_CHECK_VERSION(3,2,0) /* gtk < 3.2 */
+static gboolean
+gdk_event_get_button (const GdkEvent *event, guint *button)
+{
+ if (event->type != GDK_BUTTON_PRESS)
+ return FALSE;
+
+ *button = ((const GdkEventButton *) event)->button;
+ return TRUE;
+}
+#endif
+
#if GTK_CHECK_VERSION(3,2,0) /* gtk >= 3.2 */
#define hbox_new(box, homogeneous, spacing) \
do { \
@@ -79,6 +91,10 @@
gtk_scrolled_window_add_with_viewport (GTK_SCROLLED_WINDOW (container), child)
#endif
+#if !GTK_CHECK_VERSION(3,10,0) /* gtk < 3.10 */
+#define gdk_event_get_event_type(event) ((event)->type)
+#endif
+
#if GTK_CHECK_VERSION(3,10,0) /* gtk >= 3.10 */
#undef GTK_STOCK_DIALOG_WARNING
#define GTK_STOCK_DIALOG_WARNING "dialog-warning"
diff --git a/p2v/gui.c b/p2v/gui.c
index a23d0ea53..1fd980044 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -1329,15 +1329,20 @@ maybe_identify_click (GtkWidget *interfaces_list, GdkEventButton *event,
gpointer data)
{
gboolean ret = FALSE; /* Did we handle this event? */
+ guint button;
/* Single left click only. */
- if (event->type == GDK_BUTTON_PRESS && event->button == 1) {
+ if (gdk_event_get_event_type ((const GdkEvent *) event) == GDK_BUTTON_PRESS &&
+ gdk_event_get_button ((const GdkEvent *) event, &button) &&
+ button == 1) {
GtkTreePath *path;
GtkTreeViewColumn *column;
+ gdouble event_x, event_y;
- if (gtk_tree_view_get_path_at_pos (GTK_TREE_VIEW (interfaces_list),
- event->x, event->y,
- &path, &column, NULL, NULL)) {
+ if (gdk_event_get_coords ((const GdkEvent *) event, &event_x, &event_y)
+ && gtk_tree_view_get_path_at_pos (GTK_TREE_VIEW (interfaces_list),
+ event_x, event_y,
+ &path, &column, NULL, NULL)) {
GList *cols;
gint column_index;
--
2.17.2
5 years, 6 months
[PATCH v2 0/2] p2v: add Shutdown option
by Pino Toscano
This small series for p2v refactors the Reboot menu of the conversion
dialog into something slightly more general, and add the possibility to
shut the machine down.
Lots of work to deal with old GTK versions ...
Changes from v1:
- fix shutdown command
Pino Toscano (2):
p2v: turn Reboot button into a Shutdown popup menu button
p2v: add a Shutdown action (RHBZ#1642044)
p2v/gui.c | 119 +++++++++++++++++++++++++++++++++++++++++++----
p2v/virt-p2v.pod | 2 +-
2 files changed, 110 insertions(+), 11 deletions(-)
--
2.17.2
5 years, 6 months
Re: [Libguestfs] [Qemu-devel] How to emulate block I/O timeout on qemu side?
by Eric Blake
On 11/2/18 3:11 AM, Dongli Zhang wrote:
> Hi,
>
> Is there any way to emulate I/O timeout on qemu side (not fault injection in VM
> kernel) without modifying qemu source code?
You may be interested in Rich's work on nbdkit. If you don't mind the
overhead of the host connecting through NBD, then you can use nbdkit's
delay and fault-injection filters for inserting delays or even
run-time-controllable failures to investigate how the guest reacts to
those situations
>
> For instance, I would like to observe/study/debug the I/O timeout handling of
> nvme, scsi, virtio-blk (not supported) of VM kernel.
>
> Is there a way to trigger this on purpose on qemu side?
>
> Thank you very much!
>
> Dongli Zhang
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
5 years, 6 months
[PATCH] p2v: make-disk: rely on os-release for host distro detection
by Pino Toscano
Instead of assuming the latest Fedora version if /etc/redhat-release is
available, or the latest Debian with /etc/debian_version, use only
/etc/os-release. The possible name of the virt-builder template is
created by concatenating $ID and $VERSION_ID, which is generally a
better guess than what previously done, and better matches the host OS.
This affects only the case when os-version is not specified as command
line argument.
---
p2v/virt-p2v-make-disk.in | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/p2v/virt-p2v-make-disk.in b/p2v/virt-p2v-make-disk.in
index bdf7fd96e..ee65716f2 100644
--- a/p2v/virt-p2v-make-disk.in
+++ b/p2v/virt-p2v-make-disk.in
@@ -122,12 +122,8 @@ if [ $# -eq 1 ]; then
else
# If osversion was not set, then we must guess a good value
# based on the host distro.
- if test -f /etc/redhat-release; then
- osversion="$(virt-builder -l | sort |
- @AWK@ '/^fedora-[1-9]/ {print $1}' | tail -1)"
- elif test -f /etc/debian_version; then
- osversion="$(virt-builder -l | sort |
- @AWK@ '/^debian-[1-9]/ {print $1}' | tail -1)"
+ if test -f /etc/os-release; then
+ osversion="`. /etc/os-release && echo ${ID}-${VERSION_ID}`"
fi
if [ "x$osversion" = "x" ]; then
echo "$program: unable to guess a suitable os-version."
--
2.17.2
5 years, 6 months
[PATCH 0/2] p2v: add Shutdown option
by Pino Toscano
This small series for p2v refactors the Reboot menu of the conversion
dialog into something slightly more general, and add the possibility to
shut the machine down.
Lots of work to deal with old GTK versions ...
Pino Toscano (2):
p2v: turn Reboot button into a Shutdown popup menu button
p2v: add a Shutdown action (RHBZ#1642044)
p2v/gui.c | 119 +++++++++++++++++++++++++++++++++++++++++++----
p2v/virt-p2v.pod | 2 +-
2 files changed, 110 insertions(+), 11 deletions(-)
--
2.17.2
5 years, 6 months
Plan to fix virt-v2v Windows static IPs bug (RHBZ#1626503)
by Richard W.M. Jones
https://bugzilla.redhat.com/show_bug.cgi?id=1626503
If you use virt-v2v to convert a Windows guest then everything usually
works fine as long as the guest is using DHCP. However if it has
static IPs then that information is lost after conversion (the guest
will end up with new virtio network drivers which try to use DHCP, and
old network drivers with the static IPs that are unconnected to any
device).
We've had several attempts to solve this bug before, most recently:
(Link A) https://www.redhat.com/archives/libguestfs/2018-October/msg00022.html
I had a bit of a think about this, including playing with some
Powershell scripting (see attachment), and this is my new plan to fix
this bug.
It's important to note this will only fix a subset of cases:
(Caveat A) Only Windows >= 7 (because it requires Powershell which is
not installed by default on earlier Windows).
(Caveat B) Probably only the case where Windows has a single network
interface. This is because Windows doesn't internally save any
association between network device and MAC address, and thus there is
no way to associate network device configuration read from the Windows
Registry with a specific network device from the virt-v2v metadata
(which only knows the MAC address of the device from the outside). It
might work for multiple network interfaces but could bring the
interfaces up with the wrong static IPs.
The plan would be:
(1) During conversion we can read out the static IP address of the
network interface (see Link A above for working code). Using the same
code we can find out if Windows is using static IP or DHCP, and we
would ignore any DHCP interfaces.
(2) For Windows guests with >= 1 network adapters with static IPs,
during conversion a firstboot Powershell script is inserted in the
Windows guest. This script will know the static IP of the guest read
in step (1). It will wait for an adapter with the expected MAC
address to appear (which might happen some time after boot) and when
it appears will assign the static IP to that adapter.
What happens in the multiple static IPs case is TBD. Probably we'll
start by printing a warning.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
5 years, 6 months