Bug 920681

The "sginfo -l" aspect of this is now being tracked through bug #920687.

(commenting publicly because the bug is private already)

I think this bug has been analyzed completely by Ray (in customer portal case 00796597, associated with bug 920687 too, comment 1 here), and by Sander Keemink from the customer side.

Basically what happens is: customer runs a script periodically that invokes "sginfo -l". "sginfo" in that mode happens to open /dev/snapshot (snapshot_open() in "kernel/power/user.c"), the O_RDONLY mode calls pm_notifier_call_chain(PM_HIBERNATION_PREPARE).

In "kernel/cpu.c" that notifier sets "cpu_hotplug_disabled":

cpu_hotplug_pm_callback()
  cpu_hotplug_disable_before_freeze()
    cpu_hotplug_disabled = 1

From the other side, massaging the CPU "online" files in sysfs enters store_online() in "drivers/base/cpu.c"

store_online()
  cpu_up() [kernel/cpu.c]
    if cpu_hotplug_disabled then return -EBUSY

If the qemu guest agent met this error when hot-(un-)plugging a VCPU, it would propagate the error to libvirtd. I think the entire logic is sane, except that opening a device *maybe* shouldn't immediately emit a significant notification.

The kernel logic is identical in upstream, so this bug should be taken there first... if we (or they) consider it a bug at all. I seem to recall that just opening eg. watchdog devices is not without consequences (*), and I guess the same could be said about opening tape drives or just plain FIFOs in the filesystem (if there's a writer blocked on them, waiting for the first reader to open).

(*) Yup, see "Documentation/watchdog/watchdog-api.txt":

  The simplest API:

  All drivers support the basic mode of operation, where the watchdog
  activates as soon as /dev/watchdog is opened and will reboot unless
  the watchdog is pinged within a certain time [...]

With that in mind I'm closing this BZ as NOTABUG: "sginfo" just shouldn't indiscriminately open device nodes. It could search for names more carefully, and/or call stat() before open() and check the st_rdev field.

Ray, if you disagree with the closure, please feel free to reopen the BZ, but in that case I'll de-assign myself (as the BZ is not virt-related). Thanks.

FWIW the /dev/snapshot behavior is documented in "Documentation/power/userland-swsusp.txt":

  The interface consists of a character device providing the open(),
  release(), read(), and write() operations as well as several ioctl()
  commands defined in include/linux/suspend_ioctls.h .  The major and minor
  numbers of the device are, respectively, 10 and 231, and they can
  be read from /sys/class/misc/snapshot/dev.

  The device can be open either for reading or for writing.  If open for
  reading, it is considered to be in the suspend mode. [...]

My "/sys/class/misc/snapshot/dev" says "10:231", this even gives "sginfo" a hint what to look for in "st_rdev" with makedev()/major()/minor().

Global Support Services has privatised comments pertaining to Red Hat internal workflow considerations and has not identified sensitive data in or attached to this bugzilla. Uncloaking this bugzilla as per Engineering's suggestion to facilitate discussions with upstream (kernel and/or sg3_utils).