[libvirt] [PATCH] add cd and pwd commands to virsh
by Paolo Bonzini
This patch adds cd and pwd commands to virsh. These can be useful
together with commands that refer to files in the local file systems,
especially save and restore.
I explicitly decided not to provide any other command, e.g. mkdir,
to avoid going down a slippery slope (now you want mkdir, tomorrow
ls and rm and so on...). If anything, it would be possible to add
a generic shell command---but cd cannot be implemented that way,
so pwd shall remain the sole "exception".
I did not implement "cd -".
Ok?
2009-07-02 Paolo Bonzini <pbonzini(a)redhat.com>
* bootstrap: Add getcwd module.
* docs/virsh.pod: Document cd and pwd commands.
* src/virsh.c (info_cd, opts_cd, cmdCd, info_pwd,
cmdPwd): New.
(commands): Add cd and pwd commands.
* virsh.1: Regenerate.
Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com>
---
bootstrap | 1 +
docs/virsh.pod | 12 ++++++++
src/virsh.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
virsh.1 | 12 ++++++++-
4 files changed, 101 insertions(+), 1 deletions(-)
diff --git a/bootstrap b/bootstrap
index baf52e7..b14d8cc 100755
--- a/bootstrap
+++ b/bootstrap
@@ -69,6 +69,7 @@ c-ctype
close
connect
getaddrinfo
+getcwd
gethostname
getpass
gettext
diff --git a/docs/virsh.pod b/docs/virsh.pod
index 6434d78..9530ba6 100644
--- a/docs/virsh.pod
+++ b/docs/virsh.pod
@@ -82,6 +82,18 @@ Running hypervisor: Xen 3.0.0
=back
+=item B<cd> I<directory> optional
+
+Will change current directory to I<directory>. The default directory
+for the B<cd> command is the home directory or, if there is no I<HOME>
+variable in the environment, the root directory.
+
+This command is only available in interactive mode.
+
+=item B<pwd>
+
+Will print the current directory.
+
=item B<connect> I<URI> optional I<--readonly>
(Re)-Connect to the hypervisor. This is a build-in command after shell
diff --git a/src/virsh.c b/src/virsh.c
index 5623499..856e5a0 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -6047,6 +6047,81 @@ editReadBackFile (vshControl *ctl, const char *filename)
}
/*
+ * "cd" command
+ */
+static const vshCmdInfo info_cd[] = {
+ {"help", gettext_noop("change the current directory")},
+ {"desc", gettext_noop("Change the current directory.")},
+ {NULL, NULL}
+};
+
+static const vshCmdOptDef opts_cd[] = {
+ {"dir", VSH_OT_DATA, 0, gettext_noop("directory to switch to (default: home or else root)")},
+ {NULL, 0, 0, NULL}
+};
+
+static int
+cmdCd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+ const char *dir;
+ int found;
+
+ if (!ctl->imode) {
+ vshError(ctl, FALSE, _("cd: command valid only in interactive mode"));
+ return -1;
+ }
+
+ dir = vshCommandOptString(cmd, "dir", &found);
+ if (!found)
+ dir = getenv ("HOME");
+ if (!dir)
+ dir = "/";
+
+ if (chdir (dir) == -1) {
+ vshError(ctl, FALSE, _("cd: %s: %s"), strerror (errno), dir);
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * "pwd" command
+ */
+static const vshCmdInfo info_pwd[] = {
+ {"help", gettext_noop("print the current directory")},
+ {"desc", gettext_noop("Print the current directory.")},
+ {NULL, NULL}
+};
+
+static int
+cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+ char *cwd;
+ size_t path_max;
+ int err = TRUE;
+
+ path_max = (size_t) PATH_MAX + 2;
+ cwd = vshMalloc (ctl, path_max);
+ while (cwd) {
+ err = getcwd (cwd, path_max) == NULL;
+ if (!err || errno != ERANGE)
+ break;
+
+ path_max *= 2;
+ cwd = vshRealloc (ctl, cwd, path_max);
+ }
+
+ if (err)
+ vshError(ctl, FALSE, _("pwd: cannot get current directory: %s"), strerror (errno));
+ else
+ vshPrint (ctl, _("%s\n"), cwd);
+
+ free (cwd);
+ return !err;
+}
+
+/*
* "edit" command
*/
static const vshCmdInfo info_edit[] = {
@@ -6209,6 +6284,7 @@ static const vshCmdDef commands[] = {
{"attach-interface", cmdAttachInterface, opts_attach_interface, info_attach_interface},
{"autostart", cmdAutostart, opts_autostart, info_autostart},
{"capabilities", cmdCapabilities, NULL, info_capabilities},
+ {"cd", cmdCd, opts_cd, info_cd},
{"connect", cmdConnect, opts_connect, info_connect},
{"console", cmdConsole, opts_console, info_console},
{"create", cmdCreate, opts_create, info_create},
@@ -6277,6 +6353,7 @@ static const vshCmdDef commands[] = {
{"pool-undefine", cmdPoolUndefine, opts_pool_undefine, info_pool_undefine},
{"pool-uuid", cmdPoolUuid, opts_pool_uuid, info_pool_uuid},
+ {"pwd", cmdPwd, NULL, info_pwd},
{"quit", cmdQuit, NULL, info_quit},
{"reboot", cmdReboot, opts_reboot, info_reboot},
{"restore", cmdRestore, opts_restore, info_restore},
diff --git a/virsh.1 b/virsh.1
index 45ea614..ca03cc1 100644
--- a/virsh.1
+++ b/virsh.1
@@ -132,7 +132,7 @@
.\" ========================================================================
.\"
.IX Title "VIRSH 1"
-.TH VIRSH 1 "2009-04-16" "libvirt-0.6.2" "Virtualization Support"
+.TH VIRSH 1 "2009-07-02" "libvirt-0.6.4" "Virtualization Support"
.\" For nroff, turn off justification. Always turn off hyphenation; it makes
.\" way too many mistakes in technical documents.
.if n .ad l
@@ -214,6 +214,16 @@ Running hypervisor: Xen 3.0.0
.RE
.RS 4
.RE
+.IP "\fBcd\fR \fIdirectory\fR optional" 4
+.IX Item "cd directory optional"
+Will change current directory to \fIdirectory\fR. The default directory
+for the \fBcd\fR command is the home directory or, if there is no \fI\s-1HOME\s0\fR
+variable in the environment, the root directory.
+.Sp
+This command is only available in interactive mode.
+.IP "\fBpwd\fR" 4
+.IX Item "pwd"
+Will print the current directory.
.IP "\fBconnect\fR \fI\s-1URI\s0\fR optional \fI\-\-readonly\fR" 4
.IX Item "connect URI optional --readonly"
(Re)\-Connect to the hypervisor. This is a build-in command after shell
--
1.6.2.5
15 years, 4 months
[libvirt] consistency: push "update" hook vs. "make syntax-check"
by Jim Meyering
Currently the server side hook that runs "git diff --check"
to prevent pushing a change that adds trailing blanks is
more strict than our "make syntax-check" hook, since the former
rejects any change that adds blank lines at the end of a file,
while "make syntax-check" doesn't complain about that.
The two should be consistent.
One way is to make "make syntax-check" more strict.
If we were to do that, we'd have to choose between
cleaning existing files and exempting them from the new test.
Cleaning is easy and doesn't impact tests at all, so I prefer it.
Here's what would be involved:
- remove 121 trailing newlines from 109 files by running this command:
git ls-files -z | xargs -0 perl -pi -0777 -e 's/\n\n+$/\n/'
Add a rule to cfg.mk so that "make syntax-check" warns about
any new violations. It might run something like this:
git ls-files -z \
| xargs -0 perl -ln -0777 -e '/\n(\n+)$/ and print "$ARGV: ".length $1'
That command prints the name of each offending file with its trailing
blank line count. While it takes well under a second on my system,
(admittedly, with a hot cache), it's not well optimized, reading
each file into memory and processing it.
If it matters, we can come up with a more efficient (yet still portable)
way to compare the last two bytes of each file to "\n\n".
I went ahead and wrote a nearly-minimal script to do that.
Rather than reading/processing all 27MB of sources,
this reads just the last 2 bytes of each of the 1048 files,
comparing those bytes to "\n\n" and printing the name when
there's a match:
git ls-files -z \
| xargs -0 perl -le '
foreach my $f (@ARGV) {
open F,"<",$f or (warn "failed to open $f: $!\n"), next;
my $p = sysseek(F, -2, 2);
# seek failure probably means file has < 2 bytes; ignore
my $two;
defined $p and $p = sysread F,$two,2;
close F;
# ignore read failure
$p && $two eq "\n\n" and (print $f),$fail=1;
} END {exit defined $fail ? 1 : 0}'
However, counting minor page faults, there's little difference
(2193 before, 1976 after), but maximum memory consumption is probably
way down. I didn't measure that.
With a hot cache, the latter takes .02elapsed,
and the former takes .09 seconds.
I'm leaning towards the simplicity of the former, in spite of its cost.
I'll bet someone can come up with a simple *and* efficient script
(probably using sed) to list files with one or more trailing blank line.
15 years, 4 months
[libvirt] PATCH: Allow QEMU VMs to be run unprivileged
by Daniel P. Berrange
This patch makes it such that the privileges libvirtd daemon can
run unprivileged QEMU guests. The default remains unchanged with
QEMU running as root:root, but the package maintainer can request
an alternative default user at build time, and the sysadmin can
also override this at install time with /etc/libvirt/qemu.conf.
As well as making QEMU setuid/gid to the non-root user, this
patch takes care of chown'ing all resources it needs to access.
This currently includes
- /dev/bus/usb/$BUS/$DEVICE for any assigned USB devices
- /sys/bus/pci/$ADDR/{config,resource*,rom} for PCI devs
- All disk paths
Upon shutdown it will restore ownership to root for all of
thesem, except shared/readonly disk images
NB one minor problem is that USB devices attached based
on vendor/product ID aren't handled. Need to figure out a
way to deal with this....
Daniel
commit 5283a949f4f5cc75e471ddd624362261280378d9
Author: Daniel P. Berrange <berrange(a)redhat.com>
Date: Wed Jul 15 22:25:01 2009 +0100
Run QEMU guests as an unprivileged user
* configure.in: Add --with-qemu-user and --with-qemu-group args
* libvirt.spec.in: use 'qemu' for user/group for Fedora >= 12
* qemud/libvirtd_qemu.arg, qemud/test_libvirtd_qemu.aug,
src/qemu.conf: Add 'user' and 'group' args for configuration
* src/Makefile.am: Create %localstatedir/cache/libvirt/qemu
* src/qemu_conf.c, src/qemu_conf.h: Load user/group from config
* src/qemu_driver.c: Change user ID/group ID when launching QEMU
guests. Change user/group ownership on disks/usb/pci devs.
Put memory dumps in %localstatedir/cache/libvirt/qemu
* src/util.c, src/util.h: Add convenient APIs for converting
username/groupname to user ID / group ID
diff --git a/configure.in b/configure.in
index 552089a..634e812 100644
--- a/configure.in
+++ b/configure.in
@@ -1437,6 +1437,19 @@ AM_CONDITIONAL([WITH_NODE_DEVICES], [test "$with_nodedev" = "yes"])
AM_CONDITIONAL([WITH_LINUX], [test `uname -s` = "Linux"])
+
+AC_ARG_WITH([qemu-user],
+ [ --with-qemu-user username to run QEMU system instance as],
+ [QEMU_USER=${withval}],
+ [QEMU_USER=root])
+AC_ARG_WITH([qemu-group],
+ [ --with-qemu-group groupname to run QEMU system instance as],
+ [QEMU_GROUP=${withval}],
+ [QEMU_GROUP=root])
+AC_DEFINE_UNQUOTED([QEMU_USER], ["$QEMU_USER"], [QEMU user account])
+AC_DEFINE_UNQUOTED([QEMU_GROUP], ["$QEMU_GROUP"], [QEMU group account])
+
+
# Only COPYING.LIB is under version control, yet COPYING
# is included as part of the distribution tarball.
# Copy one to the other, but only if this is a srcdir-build.
@@ -1578,3 +1591,7 @@ AC_MSG_NOTICE([ Debug: $enable_debug])
AC_MSG_NOTICE([ Warnings: $enable_compile_warnings])
AC_MSG_NOTICE([ Readline: $lv_use_readline])
AC_MSG_NOTICE([])
+AC_MSG_NOTICE([Privileges])
+AC_MSG_NOTICE([])
+AC_MSG_NOTICE([ QEMU: $QEMU_USER:$QEMU_GROUP])
+AC_MSG_NOTICE([])
\ No newline at end of file
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6321eaa..1bde6c5 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -46,6 +46,14 @@
%define with_capng 0%{!?_without_capng:1}
%endif
+%if 0%{?fedora} >= 12
+%define qemu_user qemu
+%define qemu_group qemu
+%else
+%define qemu_user root
+%define qemu_group root
+%endif
+
#
# If building on RHEL switch on the specific support
#
@@ -309,6 +317,8 @@ of recent versions of Linux (and other OSes).
%{?_without_storage_iscsi} \
%{?_without_storage_disk} \
%{?_without_numactl} \
+ --with-qemu-user=%{qemu_user} \
+ --with-qemu-group=%{qemu_group} \
--with-init-script=redhat \
--with-qemud-pid-file=%{_localstatedir}/run/libvirt_qemud.pid \
--with-remote-file=%{_localstatedir}/run/libvirtd.pid
@@ -446,8 +456,9 @@ fi
%dir %attr(0700, root, root) %{_localstatedir}/cache/libvirt/
%if %{with_qemu}
-%dir %{_localstatedir}/run/libvirt/qemu/
-%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/qemu/
+%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/run/libvirt/qemu/
+%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/
+%dir %attr(0700, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/
%endif
%if %{with_lxc}
%dir %{_localstatedir}/run/libvirt/lxc/
diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug
index 02699d6..abd6b62 100644
--- a/qemud/libvirtd_qemu.aug
+++ b/qemud/libvirtd_qemu.aug
@@ -29,6 +29,8 @@ module Libvirtd_qemu =
| str_entry "vnc_password"
| bool_entry "vnc_sasl"
| str_entry "vnc_sasl_dir"
+ | str_entry "user"
+ | str_entry "group"
(* Each enty in the config is one of the following three ... *)
let entry = vnc_entry
diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug
index 6f38e47..f62da01 100644
--- a/qemud/test_libvirtd_qemu.aug
+++ b/qemud/test_libvirtd_qemu.aug
@@ -79,6 +79,10 @@ vnc_sasl = 1
# point to the directory, and create a qemu.conf in that location
#
vnc_sasl_dir = \"/some/directory/sasl2\"
+
+user = \"root\"
+
+group = \"root\"
"
test Libvirtd_qemu.lns get conf =
@@ -161,3 +165,7 @@ vnc_sasl_dir = \"/some/directory/sasl2\"
{ "#comment" = "point to the directory, and create a qemu.conf in that location" }
{ "#comment" = "" }
{ "vnc_sasl_dir" = "/some/directory/sasl2" }
+{ "#comment" = "" }
+{ "user"= "root" }
+{ "#comment" = "" }
+{ "group" = "root" }
\ No newline at end of file
diff --git a/src/Makefile.am b/src/Makefile.am
index 889ede4..9b662ae 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -684,6 +684,7 @@ install-exec-local:
if WITH_QEMU
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu"
endif
if WITH_LXC
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lxc"
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0534d53..59c78d5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -378,6 +378,8 @@ virRun;
virSkipSpaces;
virKillProcess;
virGetUserDirectory;
+virGetUserID;
+virGetGroupID;
# uuid.h
diff --git a/src/qemu.conf b/src/qemu.conf
index c2a53c5..3009725 100644
--- a/src/qemu.conf
+++ b/src/qemu.conf
@@ -88,3 +88,10 @@
# to 'none' instead
#
# security_driver = "selinux"
+
+
+# The user ID for QEMU processes run by the system instance
+#user = "root"
+
+# The group ID for QEMU processes run by the system instance
+#group = "root"
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 414b71b..0718f39 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -92,6 +92,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
const char *filename) {
virConfPtr conf;
virConfValuePtr p;
+ char *user;
+ char *group;
/* Setup 2 critical defaults */
if (!(driver->vncListen = strdup("127.0.0.1"))) {
@@ -186,6 +188,36 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
}
}
+ p = virConfGetValue (conf, "user");
+ CHECK_TYPE ("user", VIR_CONF_STRING);
+ if (!(user = strdup(p && p->str ? p->str : QEMU_USER))) {
+ virReportOOMError(NULL);
+ virConfFree(conf);
+ return -1;
+ }
+ if (virGetUserID(NULL, user, &driver->user) < 0) {
+ VIR_FREE(user);
+ virConfFree(conf);
+ return -1;
+ }
+ VIR_FREE(user);
+
+ p = virConfGetValue (conf, "group");
+ CHECK_TYPE ("group", VIR_CONF_STRING);
+ if (!(group = strdup(p && p->str ? p->str : QEMU_GROUP))) {
+ virReportOOMError(NULL);
+ virConfFree(conf);
+ return -1;
+ }
+
+
+ if (virGetGroupID(NULL, group, &driver->group) < 0) {
+ VIR_FREE(group);
+ virConfFree(conf);
+ return -1;
+ }
+ VIR_FREE(group);
+
virConfFree (conf);
return 0;
}
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index 175173d..fbf2ab9 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -66,6 +66,9 @@ struct qemud_driver {
int privileged;
+ uid_t user;
+ gid_t group;
+
unsigned int qemuVersion;
int nextvmid;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 342ba01..f2d021c 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -71,12 +71,11 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
/* For storing short-lived temporary files. */
-#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt"
+#define TEMPDIR LOCAL_STATE_DIR "/cache/libvirt/qemu"
#define QEMU_CMD_PROMPT "\n(qemu) "
#define QEMU_PASSWD_PROMPT "Password: "
-
static int qemudShutdown(void);
static void qemuDriverLock(struct qemud_driver *driver)
@@ -1367,6 +1366,205 @@ static int qemudDomainSetSecurityLabel(virConnectPtr conn, struct qemud_driver *
return 0;
}
+
+#ifdef __linux__
+static int qemuDomainSetHostdevUSBOwnership(virConnectPtr conn,
+ virDomainHostdevDefPtr def,
+ uid_t uid, gid_t gid)
+{
+ char *usbpath = NULL;
+
+ /* XXX what todo for USB devs assigned based on product/vendor ? Doom :-( */
+ if (!def->source.subsys.u.usb.bus ||
+ !def->source.subsys.u.usb.device)
+ return 0;
+
+ if (virAsprintf(&usbpath, "/dev/bus/usb/%03d/%03d",
+ def->source.subsys.u.usb.bus,
+ def->source.subsys.u.usb.device) < 0) {
+ virReportOOMError(conn);
+ return -1;
+ }
+
+ VIR_DEBUG("Setting ownership on %s to %d:%d", usbpath, uid, gid);
+ if (chown(usbpath, uid, gid) < 0) {
+ virReportSystemError(conn, errno, _("cannot set ownership on %s"), usbpath);
+ VIR_FREE(usbpath);
+ return -1;
+ }
+ VIR_FREE(usbpath);
+
+ return 0;
+}
+
+static int qemuDomainSetHostdevPCIOwnership(virConnectPtr conn,
+ virDomainHostdevDefPtr def,
+ uid_t uid, gid_t gid)
+{
+ char *pcidir = NULL;
+ char *file = NULL;
+ DIR *dir = NULL;
+ int ret = -1;
+ struct dirent *ent;
+
+ if (virAsprintf(&pcidir, "/sys/bus/pci/devices/%04x:%02x:%02x.%x",
+ def->source.subsys.u.pci.domain,
+ def->source.subsys.u.pci.bus,
+ def->source.subsys.u.pci.slot,
+ def->source.subsys.u.pci.function) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+
+ if (!(dir = opendir(pcidir))) {
+ virReportSystemError(conn, errno,
+ _("cannot open %s"), pcidir);
+ goto cleanup;
+ }
+
+ while ((ent = readdir(dir)) != NULL) {
+ /* QEMU device assignment requires:
+ * $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN, $PCIDIR/rom
+ */
+ if (STREQ(ent->d_name, "config") ||
+ STRPREFIX(ent->d_name, "resource") ||
+ STREQ(ent->d_name, "rom")) {
+ if (virAsprintf(&file, "%s/%s", pcidir, ent->d_name) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+ VIR_DEBUG("Setting ownership on %s to %d:%d", file, uid, gid);
+ if (chown(file, uid, gid) < 0) {
+ virReportSystemError(conn, errno, _("cannot set ownership on %s"), file);
+ goto cleanup;
+ }
+ VIR_FREE(file);
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ if (dir)
+ closedir(dir);
+ VIR_FREE(file);
+ VIR_FREE(pcidir);
+ return ret;
+}
+#endif
+
+
+static int qemuDomainSetHostdevOwnership(virConnectPtr conn,
+ virDomainHostdevDefPtr def,
+ uid_t uid, gid_t gid)
+{
+ if (def->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
+ return 0;
+
+#ifdef __linux__
+ switch (def->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ return qemuDomainSetHostdevUSBOwnership(conn, def, uid, gid);
+
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
+ return qemuDomainSetHostdevPCIOwnership(conn, def, uid, gid);
+
+ }
+ return 0;
+#else
+ qemudReportError(conn, NULL, NULL, "%s",
+ _("unable to set host device ownership on this platform"));
+ return -1;
+#endif
+
+}
+
+static int qemuDomainSetDiskOwnership(virConnectPtr conn,
+ virDomainDiskDefPtr def,
+ uid_t uid, gid_t gid)
+{
+
+ if (!def->src)
+ return 0;
+
+ VIR_DEBUG("Setting ownership on %s to %d:%d", def->src, uid, gid);
+ if (chown(def->src, uid, gid) < 0) {
+ virReportSystemError(conn, errno, _("cannot set ownership on %s"),
+ def->src);
+ return -1;
+ }
+ return 0;
+}
+
+static int qemuDomainSetDeviceOwnership(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainDeviceDefPtr def,
+ int restore)
+{
+ uid_t uid;
+ gid_t gid;
+
+ if (!driver->privileged)
+ return 0;
+
+ /* short circuit case of root:root */
+ if (!driver->user && !driver->group)
+ return 0;
+
+ uid = restore ? 0 : driver->user;
+ gid = restore ? 0 : driver->group;
+
+ switch (def->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ if (restore &&
+ (def->data.disk->readonly || def->data.disk->shared))
+ return 0;
+
+ return qemuDomainSetDiskOwnership(conn, def->data.disk, uid, gid);
+
+ case VIR_DOMAIN_DEVICE_HOSTDEV:
+ return qemuDomainSetHostdevOwnership(conn, def->data.hostdev, uid, gid);
+ }
+
+ return 0;
+}
+
+static int qemuDomainSetAllDeviceOwnership(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainDefPtr def,
+ int restore)
+{
+ int i;
+ uid_t uid;
+ gid_t gid;
+
+ if (!driver->privileged)
+ return 0;
+
+ /* short circuit case of root:root */
+ if (!driver->user && !driver->group)
+ return 0;
+
+ uid = restore ? 0 : driver->user;
+ gid = restore ? 0 : driver->group;
+
+ for (i = 0 ; i < def->ndisks ; i++) {
+ if (restore &&
+ (def->disks[i]->readonly || def->disks[i]->shared))
+ continue;
+
+ if (qemuDomainSetDiskOwnership(conn, def->disks[i], uid, gid) < 0)
+ return -1;
+ }
+
+ for (i = 0 ; i < def->nhostdevs ; i++) {
+ if (qemuDomainSetHostdevOwnership(conn, def->hostdevs[i], uid, gid) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
static virDomainPtr qemudDomainLookupByName(virConnectPtr conn,
const char *name);
@@ -1377,14 +1575,39 @@ struct gemudHookData {
};
static int qemudSecurityHook(void *data) {
- struct gemudHookData *h = (struct gemudHookData *) data;
+ struct gemudHookData *h = (struct gemudHookData *) data;
+
+ if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) {
+ qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Failed to set security label"));
+ return -1;
+ }
+
+ if (h->driver->privileged) {
+ DEBUG("Dropping privileges of VM to %d:%d", h->driver->user, h->driver->group);
+
+ if (qemuDomainSetAllDeviceOwnership(h->conn, h->driver, h->vm->def, 0) < 0)
+ return -1;
- if (qemudDomainSetSecurityLabel(h->conn, h->driver, h->vm) < 0) {
- qemudReportError(h->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Failed to set security label"));
+ if (h->driver->group) {
+ if (setregid(h->driver->group, h->driver->group) < 0) {
+ virReportSystemError(NULL, errno,
+ _("cannot change to '%d' group"),
+ h->driver->group);
return -1;
+ }
}
- return 0;
+ if (h->driver->user) {
+ if (setreuid(h->driver->user, h->driver->user) < 0) {
+ virReportSystemError(NULL, errno,
+ _("cannot change to '%d' user"),
+ h->driver->user);
+ return -1;
+ }
+ }
+ }
+
+ return 0;
}
static int
@@ -1662,6 +1885,10 @@ static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
VIR_FREE(vm->def->seclabel.imagelabel);
}
+ if (qemuDomainSetAllDeviceOwnership(conn, driver, vm->def, 1) < 0)
+ VIR_WARN("Failed to restore all device ownership for %s",
+ vm->def->name);
+
if (qemudRemoveDomainStatus(conn, driver, vm) < 0) {
VIR_WARN(_("Failed to remove domain status for %s"),
vm->def->name);
@@ -4227,12 +4454,20 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
if (driver->securityDriver)
driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk);
+
+ if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0)
+ goto cleanup;
+
ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev);
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (driver->securityDriver)
driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk);
+
+ if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0)
+ goto cleanup;
+
if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev);
} else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
@@ -4255,6 +4490,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+ if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 0) < 0)
+ goto cleanup;
+
ret = qemudDomainAttachHostDevice(dom->conn, vm, dev);
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
@@ -4267,8 +4505,11 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
ret = -1;
cleanup:
- if (ret < 0)
+ if (ret < 0) {
+ if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
+ VIR_WARN0("Fail to restore disk device ownership");
virDomainDeviceDefFree(dev);
+ }
if (vm)
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
@@ -4396,6 +4637,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
if (driver->securityDriver)
driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk);
+ if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
+ VIR_WARN0("Fail to restore disk device ownership");
}
else
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
diff --git a/src/util.c b/src/util.c
index 178ff0c..2420f8b 100644
--- a/src/util.c
+++ b/src/util.c
@@ -55,6 +55,7 @@
#include <netdb.h>
#ifdef HAVE_GETPWUID_R
#include <pwd.h>
+#include <grp.h>
#endif
#if HAVE_CAPNG
#include <cap-ng.h>
@@ -1862,4 +1863,78 @@ char *virGetUserDirectory(virConnectPtr conn,
return ret;
}
+
+
+int virGetUserID(virConnectPtr conn,
+ const char *name,
+ uid_t *uid)
+{
+ char *strbuf;
+ struct passwd pwbuf;
+ struct passwd *pw = NULL;
+ size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
+
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
+ virReportOOMError(conn);
+ return -1;
+ }
+
+ /*
+ * From the manpage (terrifying but true):
+ *
+ * ERRORS
+ * 0 or ENOENT or ESRCH or EBADF or EPERM or ...
+ * The given name or uid was not found.
+ */
+ if (getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) {
+ virReportSystemError(conn, errno,
+ _("Failed to find user record for name '%s'"),
+ name);
+ VIR_FREE(strbuf);
+ return -1;
+ }
+
+ *uid = pw->pw_uid;
+
+ VIR_FREE(strbuf);
+
+ return 0;
+}
+
+
+int virGetGroupID(virConnectPtr conn,
+ const char *name,
+ gid_t *gid)
+{
+ char *strbuf;
+ struct group grbuf;
+ struct group *gr = NULL;
+ size_t strbuflen = sysconf(_SC_GETGR_R_SIZE_MAX);
+
+ if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
+ virReportOOMError(conn);
+ return -1;
+ }
+
+ /*
+ * From the manpage (terrifying but true):
+ *
+ * ERRORS
+ * 0 or ENOENT or ESRCH or EBADF or EPERM or ...
+ * The given name or uid was not found.
+ */
+ if (getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr) != 0 || gr == NULL) {
+ virReportSystemError(conn, errno,
+ _("Failed to find group record for name '%s'"),
+ name);
+ VIR_FREE(strbuf);
+ return -1;
+ }
+
+ *gid = gr->gr_gid;
+
+ VIR_FREE(strbuf);
+
+ return 0;
+}
#endif
diff --git a/src/util.h b/src/util.h
index 6dd005f..1a7286c 100644
--- a/src/util.h
+++ b/src/util.h
@@ -217,6 +217,12 @@ int virKillProcess(pid_t pid, int sig);
#ifdef HAVE_GETPWUID_R
char *virGetUserDirectory(virConnectPtr conn,
uid_t uid);
+int virGetUserID(virConnectPtr conn,
+ const char *name,
+ uid_t *uid);
+int virGetGroupID(virConnectPtr conn,
+ const char *name,
+ gid_t *gid);
#endif
int virRandomInitialize(unsigned int seed);
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 4 months
[libvirt] [PATCH 0/9] Refactor remote protocol ready for data streams
by Daniel P. Berrange
The current libvirtd remote protocol dispatch code is written in
such a way that assumes the only incoming messages from clients
are method calls. This makes it very hard to support data streams.
This patch series does an incrmental refactoring of alot of code
to allow data streams to be easily wired in.
Daniel P. Berrange (9):
Split generic RPC message dispatch code out from remote protocol API
handlers
Decode incoming request header before invoking dispatch code
Separate code for encoding outgoing remote message headers
Change code generator to give async event messages their own postfix
Move queuing of RPC replies into dispatch code
Change the way client event loop watches are managed
Split out code for handling incoming method call messages
Define an API for registering incoming message dispatch filters
Rename 'direction' to 'type' in remote_message_header
qemud/Makefile.am | 3 +-
qemud/dispatch.c | 533 ++++++++++++++++++++++++++++++++++++
qemud/dispatch.h | 63 +++++
qemud/qemud.c | 136 ++++++----
qemud/qemud.h | 35 ++-
qemud/remote.c | 442 ++++--------------------------
qemud/remote.h | 72 +++++
qemud/remote_dispatch_prototypes.h | 7 -
qemud/remote_dispatch_ret.h | 1 -
qemud/remote_dispatch_table.h | 6 +-
qemud/remote_generate_stubs.pl | 23 ++-
qemud/remote_protocol.c | 6 +-
qemud/remote_protocol.h | 18 +-
qemud/remote_protocol.x | 66 ++++--
src/remote_internal.c | 18 +-
15 files changed, 926 insertions(+), 503 deletions(-)
create mode 100644 qemud/dispatch.c
create mode 100644 qemud/dispatch.h
create mode 100644 qemud/remote.h
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 4 months
[libvirt] PATCH: Fix free of uninitialized data on PCI device failure
by Daniel P. Berrange
The pci.c file was freeing a variable that was not initialized in its
failure path. This causes a crash if the user specifies a PCI device
for a guest which does not exist
commit 87f584fdd688865b4ea7f54d1b631f546535d8c2
Author: Daniel P. Berrange <berrange(a)redhat.com>
Date: Thu Jul 16 13:23:32 2009 +0100
Fix free of unitialized data upon PCI open fail
diff --git a/src/pci.c b/src/pci.c
index 3ffa0aa..4030a14 100644
--- a/src/pci.c
+++ b/src/pci.c
@@ -834,10 +834,8 @@ pciReadDeviceID(pciDevice *dev, const char *id_name)
dev->name, id_name);
/* ID string is '0xNNNN\n' ... i.e. 7 bytes */
- if (virFileReadAll(path, 7, &id_str) < 7) {
- VIR_FREE(id_str);
+ if (virFileReadAll(path, 7, &id_str) < 0)
return NULL;
- }
/* Check for 0x suffix */
if (id_str[0] != '0' || id_str[1] != 'x') {
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 4 months
[libvirt] Can not get domain's cpu time
by Zhang Qian
Hi,
I have a domain running in my KVM box, and try to get its vcpu info by
calling virDomainGetVcpus(), but it seems the cpu time returned to me
is always 0.
And I also found virsh can not get the CPU time too:
$ virsh vcpuinfo aaa
VCPU: 0
CPU: 0
State: running
CPU Affinity: yy
I tried the same virsh command in my Xen box for a running domain, the
output is:
$ virsh vcpuinfo test1
VCPU: 0
CPU: 1
State: running
CPU time: 322.1s <----- I need this
CPU Affinity: yy
As you see, for KVM domain, there is no "CPU time".
But it's very strange that virt-manager can show the right CPU usage
for my running domain, I do not know where virt-manger gets it.
Can anyone tell me which libvirt API should I call to get the CPU time? Thanks!
Regards,
Qian
15 years, 4 months
[libvirt] PATCH: Handle changed monitor syntax for PCI hotplug/unplug
by Daniel P. Berrange
Annoyingly when PCI hotplug support was merged from KVM into QEMU the
monitor syntax changed :-( This patch tries to deal with this by
detecting the error message, and then re-trying with the alternative
syntax.
It also ensures the monitor error mesages is include with any libvirt
error raised, so that we can more easily detect problems in the future
FYI, I have a test case for this now in the TCK
http://libvirt.org/git/?p=libvirt-tck.git;a=commit;h=0ea2a3f3c426b325f809...
Regards,
Daniel
>From fca1582a7406d9eef7ff1a5e108986df76aeb6a1 Mon Sep 17 00:00:00 2001
From: Daniel P. Berrange <berrange(a)redhat.com>
Date: Mon, 6 Jul 2009 15:58:55 +0100
Subject: [PATCH 2/3] Fix PCI device hotplug/unplug with newer QEMU
* src/qemu_driver.c: If hotplug/unplug fails, try with alternative
monitor command syntax
---
src/qemu_driver.c | 56 +++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index fdbbb56..25d446d 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -3915,6 +3915,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
char *cmd, *reply, *s;
char *safe_path;
const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus);
+ int tryAltSyntax = 0;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
@@ -3929,14 +3930,15 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
return -1;
}
+try_command:
safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
if (!safe_path) {
virReportOOMError(conn);
return -1;
}
- ret = virAsprintf(&cmd, "pci_add 0 storage file=%s,if=%s",
- safe_path, type);
+ ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
+ (tryAltSyntax ? "pci_addr=auto" : "0"), safe_path, type);
VIR_FREE(safe_path);
if (ret == -1) {
virReportOOMError(conn);
@@ -3952,17 +3954,27 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
DEBUG ("%s: pci_add reply: %s", vm->def->name, reply);
/* If the command succeeds qemu prints:
- * OK bus 0... */
-#define PCI_ATTACH_OK_MSG "OK bus 0, slot "
- if ((s=strstr(reply, PCI_ATTACH_OK_MSG))) {
- char* dummy = s;
- s += strlen(PCI_ATTACH_OK_MSG);
+ * OK bus 0, slot XXX...
+ * or
+ * OK domain 0, bus 0, slot XXX
+ */
+ if ((s = strstr(reply, "OK ")) &&
+ (s = strstr(s, "slot "))) {
+ char *dummy = s;
+ s += strlen("slot ");
if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1)
VIR_WARN("%s", _("Unable to parse slot number\n"));
+ /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */
+ /* XXX this slotnum is not persistant across restarts :-( */
+ } else if (!tryAltSyntax && strstr(reply, "Invalid pci address")) {
+ VIR_FREE(reply);
+ VIR_FREE(cmd);
+ tryAltSyntax = 1;
+ goto try_command;
} else {
qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("adding %s disk failed"), type);
+ _("adding %s disk failed: %s"), type, reply);
VIR_FREE(reply);
VIR_FREE(cmd);
return -1;
@@ -4179,6 +4191,7 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
char *cmd = NULL;
char *reply = NULL;
virDomainDiskDefPtr detach = NULL;
+ int tryAltSyntax = 0;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
@@ -4200,9 +4213,17 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
goto cleanup;
}
- if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) {
- virReportOOMError(conn);
- goto cleanup;
+try_command:
+ if (tryAltSyntax) {
+ if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
+ } else {
+ if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) {
+ virReportOOMError(conn);
+ goto cleanup;
+ }
}
if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
@@ -4212,12 +4233,19 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
}
DEBUG ("%s: pci_del reply: %s",vm->def->name, reply);
+
+ if (!tryAltSyntax &&
+ strstr(reply, "extraneous characters")) {
+ tryAltSyntax = 1;
+ goto try_command;
+ }
/* If the command fails due to a wrong slot qemu prints: invalid slot,
* nothing is printed on success */
- if (strstr(reply, "invalid slot")) {
+ if (strstr(reply, "invalid slot") ||
+ strstr(reply, "Invalid pci address")) {
qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _("failed to detach disk %s: invalid slot %d"),
- detach->dst, detach->slotnum);
+ _("failed to detach disk %s: invalid slot %d: %s"),
+ detach->dst, detach->slotnum, reply);
goto cleanup;
}
--
1.6.2.5
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
15 years, 4 months
