On Thu, 2011-06-30 at 10:53 +0300, Pekka Enberg wrote:
> On Wed, 29 Jun 2011, Sasha Levin wrote:
> > This will allow tracking instance names and sending commands
> > to specific instances if multiple instances are running.
> >
> > Signed-off-by: Sasha Levin <[email protected]>
>
> I skipped this and related patches because I really would like to hear
> what Ingo thinks about the '/var/run/kvm-tools/' pidfile mechanism.
>
> > ---
> > tools/kvm/include/kvm/kvm.h | 5 +++-
> > tools/kvm/kvm-run.c | 5 +++-
> > tools/kvm/kvm.c | 55
> > ++++++++++++++++++++++++++++++++++++++++++-
> > tools/kvm/term.c | 3 ++
> > 4 files changed, 65 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
> > index 7d90d35..5ad3236 100644
> > --- a/tools/kvm/include/kvm/kvm.h
> > +++ b/tools/kvm/include/kvm/kvm.h
> > @@ -41,9 +41,11 @@ struct kvm {
> > const char *vmlinux;
> > struct disk_image **disks;
> > int nr_disks;
> > +
> > + const char *name;
> > };
> >
> > -struct kvm *kvm__init(const char *kvm_dev, u64 ram_size);
> > +struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name);
> > int kvm__max_cpus(struct kvm *kvm);
> > void kvm__init_ram(struct kvm *kvm);
> > void kvm__delete(struct kvm *kvm);
> > @@ -61,6 +63,7 @@ bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr);
> > void kvm__pause(void);
> > void kvm__continue(void);
> > void kvm__notify_paused(void);
> > +int kvm__get_pid_by_instance(const char *name);
> >
> > /*
> > * Debugging
> > diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
> > index 0dece2d..a4abf76 100644
> > --- a/tools/kvm/kvm-run.c
> > +++ b/tools/kvm/kvm-run.c
> > @@ -69,6 +69,7 @@ static const char *network;
> > static const char *host_ip_addr;
> > static const char *guest_mac;
> > static const char *script;
> > +static const char *guest_name;
> > static bool single_step;
> > static bool readonly_image[MAX_DISK_IMAGES];
> > static bool vnc;
> > @@ -132,6 +133,8 @@ static int virtio_9p_rootdir_parser(const struct option
> > *opt, const char *arg, i
> >
> > static const struct option options[] = {
> > OPT_GROUP("Basic options:"),
> > + OPT_STRING('\0', "name", &guest_name, "guest name",
> > + "A name for the guest"),
> > OPT_INTEGER('c', "cpus", &nrcpus, "Number of CPUs"),
> > OPT_U64('m', "mem", &ram_size, "Virtual machine memory size in MiB."),
> > OPT_CALLBACK('d', "disk", NULL, "image", "Disk image", img_name_parser),
> > @@ -546,7 +549,7 @@ int kvm_cmd_run(int argc, const char **argv, const char
> > *prefix)
> >
> > term_init();
> >
> > - kvm = kvm__init(kvm_dev, ram_size);
> > + kvm = kvm__init(kvm_dev, ram_size, guest_name);
> >
> > ioeventfd__init();
> >
> > diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> > index c400c70..4f723a6 100644
> > --- a/tools/kvm/kvm.c
> > +++ b/tools/kvm/kvm.c
> > @@ -113,11 +113,60 @@ static struct kvm *kvm__new(void)
> > return kvm;
> > }
> >
> > +static void kvm__create_pidfile(struct kvm *kvm)
> > +{
> > + int fd;
> > + char full_name[PATH_MAX], pid[10];
> > +
> > + if (!kvm->name)
> > + return;
> > +
> > + mkdir("/var/run/kvm-tools", 0777);
>
> mkdir() can fail
My assumption here is that it would indeed fail most of the time, My
idea here was to try to create the directory anyway - even if it's
already there.
>
> > + sprintf(full_name, "/var/run/kvm-tools/%s.pid", kvm->name);
> > + fd = open(full_name, O_CREAT | O_WRONLY, 0666);
> > + sprintf(pid, "%u\n", getpid());
> > + if (write(fd, pid, strlen(pid)) <= 0)
> > + die("Failed creating PID file");
> > + close(fd);
> > +}
> > +
> > +static void kvm__remove_pidfile(struct kvm *kvm)
> > +{
> > + char full_name[PATH_MAX];
> > +
> > + if (!kvm->name)
> > + return;
> > +
> > + sprintf(full_name, "/var/run/kvm-tools/%s.pid", kvm->name);
> > + unlink(full_name);
>
> unlink() can fail too - dunno if it matters in practice but there
> shouldn't be harm in checking for it?
I can check for it, but what should I do if it indeed fails? The code is
located on the exit patch, so die() here would do more harm than good.
>
> > +}
> > +
> > +int kvm__get_pid_by_instance(const char *name)
> > +{
> > + int fd, pid;
> > + char pid_str[10], pid_file[PATH_MAX];
> > +
> > + sprintf(pid_file, "/var/run/kvm-tools/%s.pid", name);
> > + fd = open(pid_file, O_RDONLY);
> > + if (fd < 0)
> > + return -1;
> > +
> > + if (read(fd, pid_str, 10) == 0)
> > + return -1;
> > +
> > + pid = atoi(pid_str);
> > + if (pid < 0)
> > + return -1;
> > +
> > + return pid;
> > +}
> > +
> > void kvm__delete(struct kvm *kvm)
> > {
> > kvm__stop_timer(kvm);
> >
> > munmap(kvm->ram_start, kvm->ram_size);
> > + kvm__remove_pidfile(kvm);
> > free(kvm);
> > }
> >
> > @@ -237,7 +286,7 @@ int kvm__max_cpus(struct kvm *kvm)
> > return ret;
> > }
> >
> > -struct kvm *kvm__init(const char *kvm_dev, u64 ram_size)
> > +struct kvm *kvm__init(const char *kvm_dev, u64 ram_size, const char *name)
> > {
> > struct kvm_pit_config pit_config = { .flags = 0, };
> > struct kvm *kvm;
> > @@ -300,6 +349,10 @@ struct kvm *kvm__init(const char *kvm_dev, u64
> > ram_size)
> > if (ret < 0)
> > die_perror("KVM_CREATE_IRQCHIP ioctl");
> >
> > + kvm->name = name;
> > +
> > + kvm__create_pidfile(kvm);
> > +
> > return kvm;
> > }
> >
> > diff --git a/tools/kvm/term.c b/tools/kvm/term.c
> > index 9947223..a0cb03f 100644
> > --- a/tools/kvm/term.c
> > +++ b/tools/kvm/term.c
> > @@ -9,7 +9,9 @@
> > #include "kvm/read-write.h"
> > #include "kvm/term.h"
> > #include "kvm/util.h"
> > +#include "kvm/kvm.h"
> >
> > +extern struct kvm *kvm;
> > static struct termios orig_term;
> >
> > int term_escape_char = 0x01; /* ctrl-a is used for escape */
> > @@ -32,6 +34,7 @@ int term_getc(int who)
> > if (term_got_escape) {
> > term_got_escape = false;
> > if (c == 'x') {
> > + kvm__delete(kvm);
> > printf("\n # KVM session terminated.\n");
> > exit(1);
> > }
> > --
> > 1.7.6
> >
> >
--
Sasha.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
