Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

virtcontainers: qemu: Don't shutdown QMP from hotplug #628

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

sboeuf
Copy link

@sboeuf sboeuf commented Aug 23, 2018

The QMP shutdown is taken care of by the sandbox release, through a
call to hypervisor.disconnect(). By shutting down the QMP at the qemu
level directly, we are creating some unrecoverable errors by trying to
close an already closed channel.

This patch simply removes the faulty code, following the same design
other hotplug functions are designed.

Fixes #627

Signed-off-by: Sebastien Boeuf [email protected]

The QMP shutdown is taken care of by the sandbox release, through a
call to hypervisor.disconnect(). By shutting down the QMP at the qemu
level directly, we are creating some unrecoverable errors by trying to
close an already closed channel.

This patch simply removes the faulty code, following the same design
other hotplug functions are designed.

Fixes kata-containers#627

Signed-off-by: Sebastien Boeuf <[email protected]>
@egernst egernst added the review label Aug 23, 2018
@sboeuf
Copy link
Author

sboeuf commented Aug 23, 2018

Split from initial PR #623

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 171810 KB
Proxy: 4162 KB
Shim: 8949 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003712 KB

@codecov
Copy link

codecov bot commented Aug 24, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@3f45818). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #628   +/-   ##
=========================================
  Coverage          ?   65.18%           
=========================================
  Files             ?       84           
  Lines             ?     9832           
  Branches          ?        0           
=========================================
  Hits              ?     6409           
  Misses            ?     2766           
  Partials          ?      657

@opendev-zuul
Copy link

opendev-zuul bot commented Aug 24, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

Copy link
Member

@caoruidong caoruidong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@bergwolf
Copy link
Member

bergwolf commented Aug 24, 2018

LGTM

Approved with PullApprove

@jodh-intel
Copy link
Contributor

jodh-intel commented Aug 24, 2018

lgtm

Approved with PullApprove

@jodh-intel
Copy link
Contributor

Ignoring zuul...

@jodh-intel jodh-intel merged commit 695244a into kata-containers:master Aug 24, 2018
@egernst egernst removed the review label Aug 24, 2018
@sboeuf sboeuf added bug Incorrect behaviour stable-candidate labels Sep 12, 2018
@sboeuf
Copy link
Author

sboeuf commented Sep 12, 2018

@sboeuf please backport to stable branches.

@sboeuf
Copy link
Author

sboeuf commented Sep 15, 2018

No need to backport since the network hotplug feature introducing this bug is not part of stable-1.1 or stable-1.2.

egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
There is an issue with the waitForDevice:
this wait will first check is the device's sys entry existed in /sys directory,
and if it existed, then it would return directly. How ever, the kernel register
a new device as an order of: 1) create the sys entry in /sys; 2) mknod in devtmpfs;
3) send uevent to userspace. The problem here is when the "wait" tested the sys entry
existed and return directly, but at that time the kernel would haven't created the device
node in devtmpfs, thus the following mount failed with couldn't resolve the device path.

Thus, the correct choice for waiting the device is depends on the uevent notice, since
once the kernel sended the device "add" uevent, it had created the device node in devtmpfs
yet.

Fixes: kata-containers#628

Signed-off-by: lifupan <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Incorrect behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

virtcontainers: qemu: Don't shutdown QMP from hotplug
6 participants