On Tue, 2011-01-04 at 10:30 -0800, Chris Wright wrote:
> * Alex Williamson (alex.williamson@xxxxxxxxxx) wrote:
> > The PCI sysfs rom file is exposed read-only by default, but we need
> > to write to it to enable and disable the ROM around the read. When
> > running as root, the code works fine as is, but when running
> > de-privileged via libvirt, the fopen("r+") will fail if the file
> > doesn't have owner write permissions. libvirt already gives us
> > ownership of the file, so we can toggle this around the short
> > usage window ourselves.
> >
> > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
>
> Acked-by: Chris Wright <chrisw@xxxxxxxxxx>
>
> > ---
> >
> > hw/device-assignment.c | 17 +++++++++++------
> > 1 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> > index 8446cd4..da0a4d7 100644
> > --- a/hw/device-assignment.c
> > +++ b/hw/device-assignment.c
> > @@ -1866,16 +1866,18 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
> > return;
> > }
> >
> > - if (access(rom_file, F_OK)) {
> > - fprintf(stderr, "pci-assign: Insufficient privileges for %s\n",
> > - rom_file);
> > + /* The ROM file is typically mode 0400, ensure that it's at least 0600
> > + * for the following fopen to succeed when qemu is de-privileged. */
> > + if (chmod(rom_file, (st.st_mode & ALLPERMS) | S_IRUSR | S_IWUSR)) {
> > + fprintf(stderr, "pci-assign: Insufficient privileges for %s (%s)\n",
> > + rom_file, strerror(errno));
> > return;
> > }
> >
> > /* Write "1" to the ROM file to enable it */
> > fp = fopen(rom_file, "r+");
> > if (fp == NULL) {
> > - return;
> > + goto restore_rom;
> > }
> > val = 1;
> > if (fwrite(&val, 1, 1, fp) != 1) {
> > @@ -1895,17 +1897,20 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
> > "or load from file with romfile=\n", rom_file);
> > qemu_ram_free(dev->dev.rom_offset);
> > dev->dev.rom_offset = 0;
> > - goto close_rom;
> > + goto disable_rom;
> > }
> >
> > pci_register_bar(&dev->dev, PCI_ROM_SLOT,
> > st.st_size, 0, pci_map_option_rom);
> > -close_rom:
> > +disable_rom:
> > /* Write "0" to disable ROM */
> > fseek(fp, 0, SEEK_SET);
> > val = 0;
> > if (!fwrite(&val, 1, 1, fp)) {
>
> Nitpick...could you unify this? (!= 1, like the enabling write check)
Sure, I'll send a quick re-spin. Thanks,
Alex
> > DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
> > }
> > +close_rom:
> > fclose(fp);
> > +restore_rom:
> > + chmod(rom_file, st.st_mode & ALLPERMS);
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
