On Wed, Nov 20, 2019 at 10:51:47AM +0100, Michal Privoznik wrote: > Another weird bug appeared concerning qemu namespaces. Basically > the problem is as follows: > > 1) Issue an API that causes libvirt to create a node in domain's > namespace, say /dev/sda with 8:0 as major:minor (the API can > be attach-disk for instance). Or simply create the node from > a console by hand. > > 2) Detach the disk from qemu. > > 3) Do something that makes /dev/sda change it's minor number. Wait, what ? major/minor numbers for SCSI disks are a defined standard IIUC $ grep SCSI ./admin-guide/devices.txt | grep block 8 block SCSI disk devices (0-15) 11 block SCSI CD-ROM devices 65 block SCSI disk devices (16-31) 66 block SCSI disk devices (32-47) 67 block SCSI disk devices (48-63) 68 block SCSI disk devices (64-79) 69 block SCSI disk devices (80-95) 70 block SCSI disk devices (96-111) 71 block SCSI disk devices (112-127) 128 block SCSI disk devices (128-143) 129 block SCSI disk devices (144-159) 130 block SCSI disk devices (160-175) 131 block SCSI disk devices (176-191) 132 block SCSI disk devices (192-207) 133 block SCSI disk devices (208-223) 134 block SCSI disk devices (224-239) 135 block SCSI disk devices (240-255) IOW, /dev/sda should always be 8,0 There is, however, the possibiity of dynamically assign major/minor numbers so its possible we'll see a block device with a changable number, but AFAIK, such a block device should never be call /dev/sda !??! IOW, the commit is fine, but is this commit message really accurate ? > > 4) Try to attach the disk again. > > The problem is, in a few cases - like disk-detach - we don't > remove the corresponding /dev node from the mount namespace > (because it may be used by some other disk's backing chain). > But this creates a problem, because if the node changes its > MAJ:MIN numbers we don't propagate the change into the domain's > namespace. We do plain mknod() and ignore EEXIST which obviously > is not enough because it doesn't guarantee that the node has > updated MAJ:MIN pair. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1752978 > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/qemu/qemu_domain.c | 25 +++++++++---------------- > 1 file changed, 9 insertions(+), 16 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 262b74d1ab..f54b9b21ff 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -13203,16 +13203,14 @@ qemuDomainCreateDeviceRecursive(const char *device, > allow_noent, ttl - 1) < 0) > goto cleanup; > } else if (isDev) { > - if (create && > - mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) { > - if (errno == EEXIST) { > - ret = 0; > - } else { > + if (create) { > + unlink(devicePath); > + if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) { > virReportSystemError(errno, > _("Failed to make device %s"), > devicePath); > + goto cleanup; > } > - goto cleanup; > } > } else if (isReg) { > if (create && > @@ -13996,17 +13994,12 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED, > } else if (isDev) { > VIR_DEBUG("Creating dev %s (%d,%d)", > data->file, major(data->sb.st_rdev), minor(data->sb.st_rdev)); > + unlink(data->file); > if (mknod(data->file, data->sb.st_mode, data->sb.st_rdev) < 0) { > - /* Because we are not removing devices on hotunplug, or > - * we might be creating part of backing chain that > - * already exist due to a different disk plugged to > - * domain, accept EEXIST. */ > - if (errno != EEXIST) { > - virReportSystemError(errno, > - _("Unable to create device %s"), > - data->file); > - goto cleanup; > - } > + virReportSystemError(errno, > + _("Unable to create device %s"), > + data->file); > + goto cleanup; > } else { > delDevice = true; > } > -- > 2.23.0 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
