arm64: fix DEBUG_AGINTC
Fix kernel compile with DEBUG_AGINTC.
OK?
+--+
Carlos
Index: agintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
retrieving revision 1.15
diff -u -p -r1.15 agintc.c
--- agintc.c7 Dec 2018 21:33:28 - 1.15
+++ agintc.c8 May 2019 03:44:51 -
@@ -845,7 +845,6 @@ agintc_route(struct agintc_softc *sc, in
#ifdef DEBUG_AGINTC
printf("router %x irq %d val %016llx\n", GICD_IROUTER(irq),
irq, ci->ci_mpidr & MPIDR_AFF);
- val);
#endif
bus_space_write_8(sc->sc_iot, sc->sc_d_ioh,
GICD_IROUTER(irq), ci->ci_mpidr & MPIDR_AFF);
Re: vmd: set dhcp hostname option during netboot
On Sat, Dec 08, 2018 at 10:13:47AM +0100, Anton Lindqvist wrote:
> Hi,
> I've been trying out the new fake netboot feature in vmd. Overall, a
> great addition that removed the need for me to run dhcpd/rebound locally
> to achieve auto install. It would be convenient if the DHCP lease
> included a hostname inferred from the VM name in order to use dedicated
> response files for different VMs. Maybe this is a behavior that
> shouldn't be limited to just netboot? The res_hnok() validation is
> borrowed from dhclient.
>
> Comments? OK?
This is a cool idea.
ok ccardenas@
+--+
Carlos
>
> Index: dhcp.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 dhcp.c
> --- dhcp.c6 Dec 2018 09:20:06 - 1.7
> +++ dhcp.c8 Dec 2018 09:04:33 -
> @@ -24,6 +24,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -44,8 +45,10 @@ dhcp_request(struct vionet_dev *dev, cha
> struct packet_ctxpc;
> struct dhcp_packet req, resp;
> struct in_addr server_addr, mask, client_addr, requested_addr;
> - size_t resplen, o;
> + size_t len, resplen, o;
> uint32_t ltime;
> + struct vmd_vm *vm;
> + const char *hostname = NULL;
>
> if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header)))
> return (-1);
> @@ -108,8 +111,12 @@ dhcp_request(struct vionet_dev *dev, cha
> resp.hlen = req.hlen;
> resp.xid = req.xid;
>
> - if (dev->pxeboot)
> + if (dev->pxeboot) {
> strlcpy(resp.file, "auto_install", sizeof resp.file);
> + vm = vm_getbyvmid(dev->vm_vmid);
> + if (vm && res_hnok(vm->vm_params.vmc_params.vcp_name))
> + hostname = vm->vm_params.vmc_params.vcp_name;
> + }
>
> if ((client_addr.s_addr =
> vm_priv_addr(>vmd_cfg,
> @@ -205,6 +212,14 @@ dhcp_request(struct vionet_dev *dev, cha
> resp.options[o++] = sizeof(server_addr);
> memcpy([o], _addr, sizeof(server_addr));
> o += sizeof(server_addr);
> +
> + if (hostname != NULL) {
> + len = strlen(hostname);
> + resp.options[o++] = DHO_HOST_NAME;
> + resp.options[o++] = len;
> + memcpy([o], hostname, len);
> + o += len;
> + }
>
> resp.options[o++] = DHO_END;
>
>
Re: add more bootdevices to vmctl
On Mon, Dec 10, 2018 at 11:30:05PM +0100, Claudio Jeker wrote:
> On Mon, Dec 10, 2018 at 02:28:48PM -0800, Carlos Cardenas wrote:
> > On Mon, Dec 10, 2018 at 10:38:56PM +0100, Reyk Floeter wrote:
> > > OK reyk@
> > >
> > > Please think about the manpage.
> > >
> > > > Am 10.12.2018 um 22:35 schrieb Claudio Jeker :
> > > >
> > > > Now that fw_cfg support is in vmd it makes sense to have -B disk
> > > > and -B cdrom. Also error out if the option is not known.
> > > >
> > > > This allows to use -B cdrom to force booting from the cdrom disk image
> > > > e.g. to update the VM image.
> > > > --
> > > > :wq Claudio
> >
> > Same comments as reyk@ .
> >
> > ok ccardenas@ when man page has been updated.
> >
>
> Here the diff with man page update.
ok ccardenas@
+--+
Carlos
>
> --
> :wq Claudio
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 main.c
> --- main.c6 Dec 2018 09:23:15 - 1.50
> +++ main.c8 Dec 2018 06:59:17 -
> @@ -856,8 +856,14 @@ ctl_start(struct parse_result *res, int
> case 'B':
> if (res->bootdevice)
> errx(1, "boot device specified multiple times");
> - if (strcmp("net", optarg) == 0)
> + if (strcmp("disk", optarg) == 0)
> + res->bootdevice = VMBOOTDEV_DISK;
> + else if (strcmp("cdrom", optarg) == 0)
> + res->bootdevice = VMBOOTDEV_CDROM;
> + else if (strcmp("net", optarg) == 0)
> res->bootdevice = VMBOOTDEV_NET;
> + else
> + errx(1, "unknown boot device %s", optarg);
> break;
> case 'r':
> if (res->isopath)
> Index: vmctl.8
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.8,v
> retrieving revision 1.56
> diff -u -p -r1.56 vmctl.8
> --- vmctl.8 6 Dec 2018 09:23:15 - 1.56
> +++ vmctl.8 10 Dec 2018 21:59:25 -
> @@ -160,14 +160,27 @@ Boot the VM with the specified kernel or
> If not specified, the default is to boot using the BIOS image in
> .Pa /etc/firmware/vmm-bios .
> .It Fl B Ar device
> -Force system to boot from the specified device for the next boot.
> +Force system to boot from the specified device.
> .Ar device
> -can be set to
> +can be set to:
> +.Pp
> +.Bl -tag -width "cdrom" -compact
> +.It Ar disk
> +boot from disk.
> +.It Ar cdrom
> +boot the CD-ROM image.
> +.It Ar net
> +perform a PXE boot using the first network interface.
> +.El
> +Currently
> .Ar net
> -to perform a PXE boot using the first network interface.
> -Currently only supported when starting the VM with
> +is only supported when booting a kernel using the
> .Fl b
> -specifying a kernel image.
> +flag while
> +.Ar disk
> +and
> +.Ar cdrom
> +only work with BIOS images.
> .It Fl c
> Automatically connect to the VM console.
> .It Fl d Ar disk
Re: add more bootdevices to vmctl
On Mon, Dec 10, 2018 at 10:38:56PM +0100, Reyk Floeter wrote:
> OK reyk@
>
> Please think about the manpage.
>
> > Am 10.12.2018 um 22:35 schrieb Claudio Jeker :
> >
> > Now that fw_cfg support is in vmd it makes sense to have -B disk
> > and -B cdrom. Also error out if the option is not known.
> >
> > This allows to use -B cdrom to force booting from the cdrom disk image
> > e.g. to update the VM image.
> > --
> > :wq Claudio
Same comments as reyk@ .
ok ccardenas@ when man page has been updated.
+--+
Carlos
> >
> > Index: main.c
> > ===
> > RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> > retrieving revision 1.50
> > diff -u -p -r1.50 main.c
> > --- main.c6 Dec 2018 09:23:15 -1.50
> > +++ main.c8 Dec 2018 06:59:17 -
> > @@ -856,8 +856,14 @@ ctl_start(struct parse_result *res, int
> >case 'B':
> >if (res->bootdevice)
> >errx(1, "boot device specified multiple times");
> > -if (strcmp("net", optarg) == 0)
> > +if (strcmp("disk", optarg) == 0)
> > +res->bootdevice = VMBOOTDEV_DISK;
> > +else if (strcmp("cdrom", optarg) == 0)
> > +res->bootdevice = VMBOOTDEV_CDROM;
> > +else if (strcmp("net", optarg) == 0)
> >res->bootdevice = VMBOOTDEV_NET;
> > +else
> > +errx(1, "unknown boot device %s", optarg);
> >break;
> >case 'r':
> >if (res->isopath)
> >
>
Re: vmd(4) fw_cfg support
On Mon, Dec 10, 2018 at 05:52:43PM +0100, Claudio Jeker wrote: > This adds the fw_cfg interface that QEMU is using to pass data to the > BIOS. It implements both IO port access and DMA access. SeaBIOS will use > the latter if available. This should be useful for adding ACPI tables or > SMBIOS data. > > This requires the latest vmm-firmware (which I just commited) and the > vmm(4) diff I just sent out to work correctly. > > Since fw_cfg requires to zero out DMA memory I extended write_mem to do > this if a NULL pointer is used for buf. I felt this is something which may > be generally useful. > -- > :wq Claudio Very nice... ok ccardenas@ +--+ Carlos
Re: change reboot behaviour in vmd
On Thu, Dec 06, 2018 at 10:33:24AM +0100, Claudio Jeker wrote:
> So doing autoinstall with -B net is great but one thing I was missing is
> changing the reboot behaviour of vmd to exit at a guest reboot.
> I came up with this minimal diff that does the trick for me. Now maybe it
> would be better to have a proper flag for this instead of overloading
> vmc_bootdevice with it.
>
> What is the preferred way of doing this?
> --
> :wq Claudio
I'm ok with this.
reyk@, what are your thoughts?
+--+
Carlos
>
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 vmd.c
> --- vmd.c 4 Dec 2018 08:15:09 - 1.107
> +++ vmd.c 4 Dec 2018 09:11:51 -
> @@ -452,7 +452,8 @@ vmd_dispatch_vmm(int fd, struct privsep_
> __func__, vmr.vmr_id);
> break;
> }
> - if (vmr.vmr_result != EAGAIN) {
> + if (vmr.vmr_result != EAGAIN ||
> + vm->vm_params.vmc_bootdevice) {
> if (vm->vm_from_config)
> vm_stop(vm, 0, __func__);
> else
>
Re: vmctl wait
On Mon, Dec 03, 2018 at 06:22:14PM +0100, Claudio Jeker wrote: > This adds a feature to vmctl/vmd to wait for a VM to stop. > It is a feature usable in many situation where you wait for a VM to halt > after work is done. This is more or less vmctl stop -w without > sending the termination to the VM. > > There is only one vmctl that can wait so if a second one comes in the > previous one will terminate with an error. This is why I modified the > error path a bit in vmctl. > > -- > :wq Claudio Ok ccardenas@ +--+ Carlos
Re: vmd: add support for local inet6 interfaces
On Tue, Nov 20, 2018 at 03:24:18PM +0100, Reyk Floeter wrote: > Hi > > On Fri, Nov 16, 2018 at 05:35:03PM +0100, Reyk Floeter wrote: > > "local interface" (-L) is an amazing feature and I use it every day; > > but it is IPv4-only and now I realized that I need IPv6 too. > > > > The attached diff implements IPv6 support for local interfaces. > > > > Updated diff with feedback from kn@ and ccardenas@ (thanks, Carlos, > for testing!) with the following changes: > > - keep the auto-generated local inet6 prefix across reloads. > - clarify the manpage describing the fd00::/8 prefix > - comments and minor things > > OK? > > Reyk > I'm good with the updated diff. ok ccardenas@ +--+ Carlos
Re: vmd: add support for local inet6 interfaces
On Fri, Nov 16, 2018 at 05:35:03PM +0100, Reyk Floeter wrote: > Hi, > > "local interface" (-L) is an amazing feature and I use it every day; > but it is IPv4-only and now I realized that I need IPv6 too. > > The attached diff implements IPv6 support for local interfaces. > > A few notes and limitations: > > - Unlike the embedded IPv4 DHCP server, it does not implement a > DHCPv6/rtsol responder in vmd. It relies on a rad(8) change that I've > sent earlier today. Configuring rad is easy enough and IPv6 users are > used to jumping though extra hoops: use my rad diff and run the daemon > with "interface tap" in /etc/rad.conf. > > - It is disabled by default. You can enable it with a global option > "local inet6" (to get a runtime random fd00::/8 ULA prefix) or "local > inet6 prefix xxx::/64" (to configure your own prefix). For > simplicity, the prefix is a global and not a per-VM option. > > - Once enabled, IPv6 will be enabled and an additional IPv6 address > configured on the host's VM tap(4) interface whenever you create it > with "local interface" / -L. > > - The IPv6 address is derived from the configured prefix and the IPv4 > address of the local interface on the VM side. This way it embeds the > VM and interface Id and you can even pf af-to it to IPv4 again! > > ``` > vm_priv_ifconfig: interface tap0 address 100.64.9.2/31 > vm_priv_ifconfig: interface tap0 address fdfc:6be5:806:930a:6440:903:0:1/96 > > 100.64.9.3 > ``` > > - The resulting address is suitable for rad(8) - just run "ifconfig > vio0 inet6 autoconf" in the guest and you'll get your /96 IPv6 > address. > > ``` > vio0: > flags=208b43 > mtu 1500 > lladdr fe:e1:bb:d1:88:4f > index 1 priority 0 llprio 3 > groups: egress > media: Ethernet autoselect > status: active > inet 100.64.9.3 netmask 0xfffe > inet6 fe80::7a1f:6128:505d:4ea5%vio0 prefixlen 64 scopeid 0x1 > inet6 fdfc:6be5:806:930a:6440:903:b11c:516 prefixlen 96 autoconf > autoconfprivacy pltime 86063 vltime 604794 > inet6 fdfc:6be5:806:930a:6440:903:d457:347a prefixlen 96 autoconf > pltime 604794 vltime 2591994 > ``` > > - The only problem is that the IPv6 address is nondeterministic where > you cannot guess the VM's IPv6 address "from the outside" (32 bits of > entropy for the guest IP). It tried it with a /127 prefix but > slaacd/rad don't handle this very well as it has a 50% chance of > creating a duplicate with the host's IP. I didn't attempt to "fix" it > as it would probably be incompatible with other rtsol clients. So I > eventually decided that this is not important as I would still use the > IPv4 address to log in - the IPv6 address is primarily used for > outbound connections. > > OK? > > Reyk Niceok ccardenas@ +--+ Carlos
Re: 6.4 - RX not working on new supported BCM574xx (bnxt)
On Fri, Nov 09, 2018 at 04:35:38AM -0700, Luthing wrote: > Hello there, > > I am facing an issue with a Broadcom NIC (specs here > https://www.broadcom.com/products/ethernet-connectivity/controllers/bcm57416/#specifications). > > After some troubleshooting, I am not able to resolve listen ARP request from > my connected switch. > The NIC has negociated 10G with auto neg enabled on the switch. > > The switch is able to see mac address on the port where the BCM NIC is > connected to. The switch can see ARP request going though him from the NIC. > But, my OBSD cannot receive packets. > > I checked with a tcpdump -i bnxt0, and I see that I am sending ARP requests, > but not receiving any response. > > A arp -a shows an incomplete mac address. > > > "dmesg | grep bn" output is : > bnxt0 at pci6 dev 0 function 0 "Broadcom BCM57416" rev 0x01: fw ver > 20.8.163, apic 10 int 2, address 01:23:45:67:89 > bnxt1 at pci6 dev 0 function 0 "Broadcom BCM57416" rev 0x01: fw ver > 20.8.163, apic 10 int 5, address 01:23:45:67:8a > bnxt0 at pci6 dev 0 function 0 "Broadcom BCM57416" rev 0x01: fw ver > 20.8.163, apic 10 int 2, address 01:23:45:67:89 > bnxt1 at pci6 dev 0 function 0 "Broadcom BCM57416" rev 0x01: fw ver > 20.8.163, apic 10 int 5, address 01:23:45:67:8a > > > Does anyone is using this NIC? > Can you help me on this? > > Thanks > Luthing With a BCM57404 (10/25 SFP28), there are times I observe this after boot. When this happens, I power off the board and then do a cold boot. We're still trying to track down root cause on this as it doesn't happen on all NICs supported by this driver. +--+ Carlos
Re: NMEA: add more gps sensor values (altitude, precision...)
On Sat, Nov 03, 2018 at 05:00:47PM +0100, Landry Breuil wrote: > Hi, > > here's a diff to add 5 new sensor values to nmea: altitude, quality, > hdop, vdop & pdop. altitude and quality are provided by GGA messages: > http://aprs.gids.nl/nmea/#gga, quality is either 0 (no fix), 1 (gps fix) > or 2 (dgps fix). > > The last 3 are 'Dilution of precision' values, respectively horizontal, > vertical & positional (ie 3d), cf > https://en.wikipedia.org/wiki/Dilution_of_precision_(navigation) & > http://www.trakgps.com/en/index.php/information/gps-articles-information/65-gps-accuracy > and are provided by GSA messages: http://aprs.gids.nl/nmea/#gsa it is > generally considered good precision when the DOP is below 2, so i've set > the sensor status warning accordingly. > > this provides for example (angle values hidden for 'privacy', just after > the gps got a fix): > nmea0.indicator0 OnOKSignal > nmea0.raw0 1 rawOKGPS fix > nmea0.raw1 1920 rawOKHDOP > nmea0.raw2 2520 raw WARNING VDOP > nmea0.raw3 3170 raw WARNING PDOP > nmea0.timedelta024.867 msOKGPS autonomous > nmea0.angle0 xx. degreesOKLatitude > nmea0.angle1 z. degreesOKLongitude > nmea0.distance0 371.50 mmOKAltitude > > and after a while, when precision improved: > nmea0.indicator0 OnOKSignal > nmea0.raw0 1 rawOKGPS fix > nmea0.raw1800 rawOKHDOP > nmea0.raw2 1300 rawOKVDOP > nmea0.raw3 1530 rawOKPDOP > nmea0.timedelta0 -296.499 msOKGPS autonomous > nmea0.angle0 xx. degreesOKLatitude > nmea0.angle1 z. degreesOKLongitude > nmea0.distance0 355.50 mmOKAltitude > > two problems with this display: > - DOPs are usually decimal values, ie 1.3, 1.53.. but raw sensors only > support integers, hence *1000. > > - GGA messages provide altitude as '355.5' in meters (i *think* the unit > is fixed in the protocol, because altitude in feets is provided by > PGRMZ) - and we only have a SENSOR_DISTANCE type in mm (which doesnt > seem used by any driver..). Thus altitude shown in milimeters.. > This sensor types were added in > https://github.com/openbsd/src/commit/d95200b806051887b8c69294833e22dad6302828 > but no driver apparently makes use of it. > > From that point, questions: > - should i add more 'interesting' sensor values, like amount of satellites > seen/used ? Yes, num sats is as quality indicator to show how much error is in the gps fix. > > - i want to add speed value (as RMC has it in knots?, and VTG in various > units, per http://aprs.gids.nl/nmea/#vtg), but we only have > SENSOR_ACCEL type in m.s-2 (which is only used by asmc(4)). Should we > add a 'speed' sensor type ? in m/s ? in km/h ? knots ? Hmmm...what's the common case for movement: are we walking (m/s), driving (km/h), or on a boat (knots)? > > - should i add a SENSOR_ALTITUDE type in meters ? Yes. > > - is there any interest in all this, from the sensors framework POV ? I think it'll be niffty to be able to trigger on events via sensord. +--+ Carlos > Otherwise i can leave it as is and just use gpsd in userspace, which > works 100% fine... > > All this done with an > umodem0 at uhub1 port 1 configuration 1 interface 0 "u-blox AG - > www.u-blox.com u-blox GNSS receiver" rev 1.10/2.01 addr 3 > > which works fine with ldattach & gpsd, ie i run doas gpsd -N -D2 $(doas > ldattach -p nmea /dev/cuaU0) and gpsmon shows me the msgs received by > the device, sent to the kernel via ldattach, and forwarded to gpsd. > > comments welcome, diff not to be commited as is of course (nmea_atoi is > *horrible*, i know..) > > Landry
Re: faq16: remove unneeded vether(4) from bridging example
On Tue, Oct 30, 2018 at 08:43:01PM +0100, Klemens Nanni wrote: > Prompted by a question on the internet, the following diff removes the > useless virtual ethernet interface from > "Option 4 - VMs as real hosts on the same network" where guests are > bridged directly into the host network. > > vether(4) serves no purpose in this setup, the egress interface can/will > be used to communicate with guests. > > As florian@ noted, this is probably a copy/pasta left-over from > "Option 3". > > Feedback? Objections? OK? > Ok ccardenas@ +--+ Carlos
Re: Reuse VM ids.
On Tue, Oct 23, 2018 at 10:21:08PM -0700, Ori Bernstein wrote: > On Mon, 8 Oct 2018 07:59:15 -0700, Bob Beck wrote: > > > works here and I like it. but probably for after unlock > > > > It's after unlock -- pinging for OKs. > > -- > Ori Bernstein > ok ccardenas@ +--+ Carlos
update magic file for qcow
Attached is patch from netbsd for updated qcow definitions. Comments? Ok? +--+ Carlos Index: msdos === RCS file: /home/los/cvs/src/usr.bin/file/magdir/msdos,v retrieving revision 1.6 diff -u -p -r1.6 msdos --- msdos 29 Jan 2016 11:50:40 - 1.6 +++ msdos 3 Oct 2018 05:25:21 - @@ -641,43 +641,77 @@ # # Qemu Emulator Images # Lines written by Friedrich Schwittay (f.schwit...@yousable.de) -# Made by reading sources and doing trial and error on existing -# qcow files -0 string QFI Qemu Image, Format: Qcow +# Updated by Adam Buchbinder (adam.buchbin...@gmail.com) +# Made by reading sources, reading documentation, and doing trial and error +# on existing QCOW files +0 string/bQFI\xFB QEMU QCOW Image # Uncomment the following line to display Magic (only used for debugging # this magic number) -#>0 string x , Magic: %s +#>0string/bx , Magic: %s -# There are currently 2 Versions: "1" and "2" -# I do not use Version 2 and therefore branch here -# but can assure: it works (tested on both versions) -# Also my Qemu 0.9.0 which uses this Version 2 refuses -# to start in its bios ->0x04 belong 2 , Version: 2 ->0x04 belong 1 , Version: 1 +# There are currently 2 Versions: "1" and "2". +# http://www.gnome.org/~markmc/qcow-image-format-version-1.html +>4 belong 1 (v1) -# Using the existence of the Backing File Offset to Branch or not +# Using the existence of the Backing File Offset to determine whether # to read Backing File Information ->>0xcbelong >0 , Backing File( Offset: %lu ->>>(0xc.L) string >\0 , Path: %s - -# Didn't get the trick here how qemu stores the "Size" at this Position -# There is actually something stored but nothing makes sense -# The header in the sources talks about it -#>>>16 lelong x , Size: %lu +>>12 belong >0 \b, has backing file ( +# Note that this isn't a null-terminated string; the length is actually +# (16.L). Assuming a null-terminated string happens to work usually, but it +# may spew junk until it reaches a \0 in some cases. +>>>(12.L) string >\0 \bpath %s # Modification time of the Backing File # Really useful if you want to know if your backing # file is still usable together with this image ->>>20bedate x , Mtime: %s ) +20 bedate >0 \b, mtime %s) +20 default x \b) + +# Size is stored in bytes in a big-endian u64. +>>24 bequad x\b, %lld bytes -# Don't know how to calculate in Magicfiles -# Also: this Information is not reliably -# stored in image-files ->>24 lelong x , Disk Size could be: %d * 256 bytes +# 1 for AES encryption, 0 for none. +>>36 belong 1 \b, AES-encrypted -0 string QEVMQEMU's suspend to disk image +# http://www.gnome.org/~markmc/qcow-image-format.html +>4 belong 2 (v2) +# Using the existence of the Backing File Offset to determine whether +# to read Backing File Information +>>8bequad >0 \b, has backing file +# Note that this isn't a null-terminated string; the length is actually +# (16.L). Assuming a null-terminated string happens to work usually, but it +# may spew junk until it reaches a \0 in some cases. Also, since there's no +# .Q modifier, we just use the bottom four bytes as an offset. Note that if +# the file is over 4G, and the backing file path is stored after the first 4G, +# the wrong filename will be printed. (This should be (8.Q), when that syntax +# is introduced.) +>>>(12.L) string >\0 (path %s) +>>24 bequad x \b, %lld bytes +>>32 belong 1 \b, AES-encrypted + +>4 belong 3 (v3) +# Using the existence of the Backing File Offset to determine whether +# to read Backing File Information +>>8bequad >0 \b, has backing file +# Note that this isn't a null-terminated string; the length is actually +# (16.L). Assuming a null-terminated string happens to work usually, but it +# may spew junk until it reaches a \0 in some cases. Also, since there's no +# .Q modifier, we just use the bottom four bytes as an offset. Note that if +# the file is over 4G, and the backing file path is stored after the first 4G, +# the wrong filename will be printed. (This should be (8.Q), when that syntax +# is introduced.) +>>>(12.L) string >\0 (path %s) +>>24 bequad x \b, %lld bytes +>>32 belong 1 \b, AES-encrypted + +>4 default x (unknown version) + +0 string/bQEVMQEMU suspend to disk image + +# QEMU QED Image +# http://wiki.qemu.org/Features/QED/Specification +0 string/bQED\0 QEMU QED Image 0 string Bochs\ Virtual\ HD\ Image Bochs disk image, >32string x type %s,
Re: vmd cores
On Wed, Oct 03, 2018 at 10:56:28AM -0700, Greg Steuck wrote: > Hi Mike, > > I'm getting core files from vmds. Here's the most recent one. Should I > start collecting more stack traces and sending them to you? > > ci-openbsd$ doas /usr/local/bin/egdb /syzkaller/src/usr.sbin/vmd/obj/vmd > /var/crash/vmd/89501.core > Reading symbols from /syzkaller/src/usr.sbin/vmd/obj/vmd...done. > [New process 178128] > [New process 294426] > [New process 350865] > Core was generated by `vmd'. > Program terminated with signal SIGSEGV, Segmentation fault. > #0 0x0c07a64148bd in virtio_shutdown (vm=0xc09e1418000) at > /syzkaller/src/usr.sbin/vmd/virtio.c:2018 > 2018vioscsi->file.close(vioscsi->file.p, 0); Greg, Can you test out the patch attached? +--+ Carlos > [Current thread is 1 (process 178128)] > (gdb) where > #0 0x0c07a64148bd in virtio_shutdown (vm=0xc09e1418000) at > /syzkaller/src/usr.sbin/vmd/virtio.c:2018 > #1 0x0c07a640cb0a in start_vm (vm=0xc09e1418000, fd=) > at /syzkaller/src/usr.sbin/vmd/vm.c:376 > #2 0x0c07a640c09e in vmm_start_vm (imsg=, > id=0x7f7e75e4, pid=0x7f7e75e0) at > /syzkaller/src/usr.sbin/vmd/vmm.c:686 > #3 0x0c07a640b7eb in vmm_dispatch_parent (fd=, > p=, imsg=0x7f7e7d58) at > /syzkaller/src/usr.sbin/vmd/vmm.c:299 > #4 0x0c07a6408b2f in proc_dispatch (fd=3, event=, > arg=0xc09d0d61000) at /syzkaller/src/usr.sbin/vmd/proc.c:660 > #5 0x0c09f915c64d in event_process_active (base=) at > /usr/src/lib/libevent/event.c:350 > #6 event_base_loop (base=0xc0a4b11e800, flags=0) at > /usr/src/lib/libevent/event.c:502 > #7 0x0c07a6409538 in proc_run (ps=0xc0a4b11a000, p=0xc07a6633080 > , procs=0xc07a6633160 , nproc=1, > run=0xc07a640af80 , arg=0x0) at > /syzkaller/src/usr.sbin/vmd/proc.c:602 > #8 0x0c07a640850b in proc_init (ps=0xc09e1418000, procs=0xc07a6633000 > , nproc=3, debug=-538846004, argc=1258557984, argv=0x0, > proc_id=PROC_VMM) at /syzkaller/src/usr.sbin/vmd/proc.c:260 > #9 0x0c07a6403a1d in main (argc=, argv=0x7f7e8008) > at /syzkaller/src/usr.sbin/vmd/vmd.c:812 > > Thanks > Greg > -- > nest.cx is Gmail hosted, use PGP for anything private. Key: > http://goo.gl/6dMsr > Fingerprint: 5E2B 2D0E 1E03 2046 BEC3 4D50 0B15 42BD 8DF5 A1B0 Index: virtio.c === RCS file: /home/los/cvs/src/usr.sbin/vmd/virtio.c,v retrieving revision 1.70 diff -u -p -r1.70 virtio.c --- virtio.c28 Sep 2018 12:35:32 - 1.70 +++ virtio.c3 Oct 2018 18:28:05 - @@ -2015,7 +2015,9 @@ virtio_shutdown(struct vmd_vm *vm) int i; /* ensure that our disks are synced */ - vioscsi->file.close(vioscsi->file.p, 0); + if (vioscsi != NULL) + vioscsi->file.close(vioscsi->file.p, 0); + for (i = 0; i < nr_vioblk; i++) vioblk[i].file.close(vioblk[i].file.p, 0); }
Re: add vlan and trunk to arm64 RAMDISK
On Fri, Sep 28, 2018 at 10:30:21AM +0200, Mark Kettenis wrote: > > Date: Thu, 27 Sep 2018 21:55:40 -0700 > > From: Carlos Cardenas > > > > Howdy. > > > > Attached is a patch to add vlan and trunk to arm64's RAMDISK (parity > > with amd64). > > > > make release'ed and tested, size increase as follows: > > > > -rw--- 1 los los12940598 Sep 27 21:01 bsd.rd-patch > > > > -rw--- 1 los los12864682 Sep 27 05:41 bsd.rd-snap > > > > > > ~74KB difference > > > > Snap - OpenBSD 6.4-beta (RAMDISK) #475: Wed Sep 26 13:15:25 MDT 2018 > > /dev/vnd1c 7.7M7.3M382K95%/mnt > > /dev/vnd1c 7907 7526 38195% 1721874 8% > > /mnt > > > > /dev/rd0a15815 15055 76095%/ > > /dev/rd0a15815 15055 76095% 1751871 9% / > > > > Patch > > > > /dev/vnd0c 7.7M7.3M382K95%/mnt > > /dev/vnd0c 7907 7526 38195% 1721874 8% > > /mnt > > > > /dev/rd0a15815 15055 76095%/ > > /dev/rd0a15815 15055 76095% 1751871 9% / > > > > Ok? > > There is a tab vs. spaces issue. Otherwise ok kettenis@ Will fix prior to commit. +--+ Carlos > > > Index: RAMDISK > > === > > RCS file: /cvs/src/sys/arch/arm64/conf/RAMDISK,v > > retrieving revision 1.74 > > diff -u -p -r1.74 RAMDISK > > --- RAMDISK 18 Sep 2018 13:45:09 - 1.74 > > +++ RAMDISK 28 Sep 2018 04:48:57 - > > @@ -251,6 +251,8 @@ islrtc* at iic? # ISL1208 RTC > > rkpmic*at iic? # RK808 PMIC > > > > pseudo-device loop 1 > > +pseudo-device vlan > > +pseudo-device trunk > > pseudo-device bpfilter 1 > > pseudo-device rd 1 > > pseudo-device bio 1 > >
add vlan and trunk to arm64 RAMDISK
Howdy. Attached is a patch to add vlan and trunk to arm64's RAMDISK (parity with amd64). make release'ed and tested, size increase as follows: -rw--- 1 los los12940598 Sep 27 21:01 bsd.rd-patch -rw--- 1 los los12864682 Sep 27 05:41 bsd.rd-snap ~74KB difference Snap - OpenBSD 6.4-beta (RAMDISK) #475: Wed Sep 26 13:15:25 MDT 2018 /dev/vnd1c 7.7M7.3M382K95%/mnt /dev/vnd1c 7907 7526 38195% 1721874 8% /mnt /dev/rd0a15815 15055 76095%/ /dev/rd0a15815 15055 76095% 1751871 9% / Patch /dev/vnd0c 7.7M7.3M382K95%/mnt /dev/vnd0c 7907 7526 38195% 1721874 8% /mnt /dev/rd0a15815 15055 76095%/ /dev/rd0a15815 15055 76095% 1751871 9% / Ok? +--+ Carlos Index: RAMDISK === RCS file: /cvs/src/sys/arch/arm64/conf/RAMDISK,v retrieving revision 1.74 diff -u -p -r1.74 RAMDISK --- RAMDISK 18 Sep 2018 13:45:09 - 1.74 +++ RAMDISK 28 Sep 2018 04:48:57 - @@ -251,6 +251,8 @@ islrtc* at iic? # ISL1208 RTC rkpmic*at iic? # RK808 PMIC pseudo-device loop 1 +pseudo-device vlan +pseudo-device trunk pseudo-device bpfilter 1 pseudo-device rd 1 pseudo-device bio 1
Re: vmd: add some NULL checks after {c,m}alloc()
On Fri, Sep 14, 2018 at 02:50:18PM +0800, Michael Mikonos wrote: > On Thu, Sep 13, 2018 at 11:10:50PM -0700, Ori Bernstein wrote: > > On Fri, 14 Sep 2018 13:50:40 +0800, Michael Mikonos wrote: > > > > > On Thu, Sep 13, 2018 at 10:08:16PM -0700, Ori Bernstein wrote: > > > > On Thu, 13 Sep 2018 10:30:54 +0800, Michael Mikonos wrote: > > > > > > > I think a check for !disk->base belongs here as done above for disk->l1. > > > > > > > I think that co > > OK miko@, in case someone with a current vmd setup wants to commit this. I'll take a look at it tonight. +--+ Carlos
vmd fail fast on unknown disk format
Howdy.
Attached patch allows vmd to fail fast on unknown disk format along with
some minor clean up on logging.
Comments? Ok?
+--+
Carlos
Index: vioqcow2.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/vioqcow2.c,v
retrieving revision 1.1
diff -u -p -r1.1 vioqcow2.c
--- vioqcow2.c 9 Sep 2018 04:09:32 - 1.1
+++ vioqcow2.c 11 Sep 2018 02:42:11 -
@@ -127,7 +127,7 @@ virtio_init_qcow2(struct virtio_backing
if (diskp == NULL)
return -1;
if (qc2_open(diskp, fd) == -1) {
- log_warnx("could not open qcow2 disk");
+ log_warnx("%s: could not open qcow2 disk", __func__);
free(diskp);
return -1;
}
@@ -162,11 +162,11 @@ qc2_open(struct qcdisk *disk, int fd)
int version;
if (pread(fd, , sizeof header, 0) != sizeof header) {
- log_warn("short read on header");
+ log_warn("%s: short read on header", __func__);
return -1;
}
if (strncmp(header.magic, "QFI\xfb", 4) != 0) {
- log_warn("invalid magic numbers");
+ log_warn("%s: invalid magic numbers", __func__);
return -1;
}
pthread_rwlock_init(>lock, NULL);
@@ -196,7 +196,7 @@ qc2_open(struct qcdisk *disk, int fd)
* We only know about the dirty or corrupt bits here.
*/
if (disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT)) {
- log_warn("%s: unsupported features %llx", __progname,
+ log_warn("%s: unsupported features %llx", __func__,
disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT));
return -1;
}
@@ -211,7 +211,7 @@ qc2_open(struct qcdisk *disk, int fd)
disk->l1[i] = be64toh(disk->l1[i]);
version = be32toh(header.version);
if (version != 2 && version != 3) {
- log_warn("%s: unknown qcow2 version %d", __progname, version);
+ log_warn("%s: unknown qcow2 version %d", __func__, version);
return -1;
}
@@ -222,16 +222,16 @@ qc2_open(struct qcdisk *disk, int fd)
* FIXME: we need to figure out a way of opening these things,
* otherwise we just crash with a pledge violation.
*/
- log_warn("unsupported external snapshot images");
+ log_warn("%s: unsupported external snapshot images", __func__);
return -1;
if (backingsz >= sizeof basepath - 1) {
- log_warn("%s: snapshot path too long", __progname);
+ log_warn("%s: snapshot path too long", __func__);
return -1;
}
if (pread(fd, basepath, backingsz, backingoff) != backingsz) {
log_warn("%s: could not read snapshot base name",
- __progname);
+ __func__);
return -1;
}
basepath[backingsz] = 0;
@@ -243,7 +243,7 @@ qc2_open(struct qcdisk *disk, int fd)
}
if (disk->base->clustersz != disk->clustersz) {
log_warn("%s: all disks must share clustersize",
- __progname);
+ __func__);
free(disk->base);
return -1;
}
@@ -414,7 +414,7 @@ xlate(struct qcdisk *disk, off_t off, in
if (inplace)
*inplace = !!(cluster & QCOW2_INPLACE);
if (cluster & QCOW2_COMPRESSED) {
- log_warn("%s: compressed clusters unsupported", __progname);
+ log_warn("%s: compressed clusters unsupported", __func__);
goto err;
}
pthread_rwlock_unlock(>lock);
@@ -556,7 +556,7 @@ inc_refs(struct qcdisk *disk, off_t off,
l2cluster = disk->end;
disk->end += disk->clustersz;
if (ftruncate(disk->fd, disk->end) < 0) {
- log_warn("refs block grow fail ");
+ log_warn("%s: refs block grow fail", __func__);
return -1;
}
buf = htobe64(l2cluster);
@@ -573,7 +573,7 @@ inc_refs(struct qcdisk *disk, off_t off,
}
refs = htobe16(refs);
if (pwrite(disk->fd, , sizeof refs, l2cluster + 2*l2idx) != 2) {
- log_warn("could not write ref block");
+ log_warn("%s: could not write ref block", __func__);
}
return 0;
}
Index: virtio.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.65
diff -u -p -r1.65 virtio.c
--- virtio.c9 Sep 2018 04:09:32 - 1.65
+++ virtio.c11 Sep 2018 02:51:37 -
@@
Enabled bnxt on arm64
Howdy. Attached is a patch to enable bnxt on arm64. Tested (and running) on mcbin with Broadcom BCM57404 (Dell variant). Comments? Ok? +--+ Carlos Index: GENERIC === RCS file: /home/los/cvs/src/sys/arch/arm64/conf/GENERIC,v retrieving revision 1.90 diff -u -p -r1.90 GENERIC --- GENERIC 27 Aug 2018 21:09:47 - 1.90 +++ GENERIC 11 Sep 2018 02:00:16 - @@ -180,6 +180,7 @@ ppb*at pci? # PCI-PCI bridges pci* at ppb? # PCI Ethernet +bnxt* at pci? # Broadcom BCM573xx, BCM574xx em*at pci? # Intel Pro/1000 Ethernet ix*at pci? # Intel 82598EB 10Gb Ethernet mskc* at pci? # Marvell Yukon-2 Index: RAMDISK === RCS file: /home/los/cvs/src/sys/arch/arm64/conf/RAMDISK,v retrieving revision 1.72 diff -u -p -r1.72 RAMDISK --- RAMDISK 27 Aug 2018 20:05:06 - 1.72 +++ RAMDISK 11 Sep 2018 02:00:14 - @@ -171,6 +171,7 @@ ppb*at pci? # PCI-PCI bridges pci* at ppb? # PCI Ethernet +bnxt* at pci? # Broadcom BCM573xx, BCM574xx em*at pci? # Intel Pro/1000 Ethernet ix*at pci? # Intel 82598EB 10Gb Ethernet mskc* at pci? # Marvell Yukon-2
Re: vmd/vmctl: allow to boot cdrom-only VMs
On Wed, Aug 22, 2018 at 08:35:23PM +0200, Reyk Floeter wrote: > Hi, > > vmctl doesn't allow to boot VMs with only a CDROM. I see no reason > for it and vmd already allows CDROM-only. > > OK? ok ccardenas
Re: [PATCH] Pluggable disk formats for vmd (qcow2 preparation)
On Sun, Aug 19, 2018 at 11:46:12PM -0700, Ori Bernstein wrote:
> One more minor update to this patch:
>
> - Remove unused enum
> - Remove unused function prototype
> - Move some qcow2-specific headers into the qcow2 patch.
Ori, it's nice seeing good progress on this. I have a couple of
questions inline below.
+--+
Carlos
>
> +/*
> + * Initializes a raw disk image backing file from an fd.
> + * Stores the number of 512 byte sectors in *szp,
> + * returning -1 for error, 0 for success.
> + */
> +int
> +virtio_init_raw(struct virtio_backing *file, off_t *szp, int fd)
> +{
> + off_t sz;
> + int *fdp;
> +
> + sz = lseek(fd, 0, SEEK_END);
> + if (sz == -1)
> + return -1;
> +
> + fdp = malloc(sizeof(int));
> + *fdp = fd;
> + file->p = fdp;
> + file->pread = raw_pread;
> + file->pwrite = raw_pwrite;
> + file->close = raw_close;
> + *szp = sz / 512;
> + return 0;
> +}
Why are we making sz represent the number of 512 byte sectors? This may
be true for hd disks but it needs to be *bytes* for ISO/cdrom.
[snip]
> @@ -1944,12 +1957,12 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
> *child_disks,
> + sizeof(uint16_t) * (2 + VIOSCSI_QUEUE_SIZE));
> vioscsi->vq[i].last_avail = 0;
> }
> - sz = lseek(child_cdrom, 0, SEEK_END);
> - vioscsi->fd = child_cdrom;
> + if (virtio_init_disk(>file, >sz,
> + child_cdrom) == -1)
> + return;
> vioscsi->locked = 0;
> vioscsi->lba = 0;
> - vioscsi->sz = sz;
> - vioscsi->n_blocks = sz >> 11; /* num of 2048 blocks in file */
> + vioscsi->n_blocks = vioscsi->sz >> 11; /* num of 2048 blocks in
> file */
This has to be the number of bytes shifted to get the number of 2K
blocks. As the patch is, this yields the incorrect number of blocks an
ISO has. This breaks ISO/cdrom support.
> vioscsi->max_xfer = VIOSCSI_BLOCK_SIZE_CDROM;
> vioscsi->pci_id = id;
> vioscsi->vm_id = vcp->vcp_id;
[snip]
> +struct virtio_backing {
> + void *p;
> + char *path;
> + ssize_t (*pread)(void *p, char *buf, size_t len, off_t off);
> + ssize_t (*pwrite)(void *p, char *buf, size_t len, off_t off);
> + void (*close)(void *p);
> +};
What is path used for? I don't see it used anywhere in this patch or in
the qcow2 patch.
vmctl: add unveil
Patch to unveil vmctl.
Comments/OK?
+--+
Carlos
Index: main.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c 12 Jul 2018 14:53:37 - 1.39
+++ main.c 18 Aug 2018 23:22:39 -
@@ -160,7 +160,7 @@ parse(int argc, char *argv[])
if (!ctl->has_pledge) {
/* pledge(2) default if command doesn't have its own pledge */
- if (pledge("stdio rpath exec unix getpw", NULL) == -1)
+ if (pledge("stdio rpath exec unix getpw unveil", NULL) == -1)
err(1, "pledge");
}
if (ctl->main(, argc, argv) != 0)
@@ -185,6 +185,8 @@ vmmaction(struct parse_result *res)
unsigned int flags;
if (ctl_sock == -1) {
+ if (unveil(SOCKET_NAME, "r") == -1)
+ err(1, "unveil");
if ((ctl_sock = socket(AF_UNIX,
SOCK_STREAM|SOCK_CLOEXEC, 0)) == -1)
err(1, "socket");
@@ -477,6 +479,10 @@ ctl_create(struct parse_result *res, int
paths[0] = argv[1];
paths[1] = NULL;
+
+ if (unveil(paths[0], "rwc") == -1)
+ err(1, "unveil");
+
if (pledge("stdio rpath wpath cpath", NULL) == -1)
err(1, "pledge");
argc--;
@@ -759,6 +765,8 @@ __dead void
ctl_openconsole(const char *name)
{
closefrom(STDERR_FILENO + 1);
+ if (unveil(VMCTL_CU, "x") == -1)
+ err(1, "unveil");
execl(VMCTL_CU, VMCTL_CU, "-l", name, "-s", "115200", (char *)NULL);
err(1, "failed to open the console");
}
Re: vmd send nameserver only once
On Wed, Aug 15, 2018 at 03:43:06PM +0200, Martijn van Duren wrote: > When running vmd with a local interface it sends the nameservers twice, > which seems a bit redundant to me and always annoys me when editing > resolv.conf manually inside the vm. > Diff below removes one of the two instances. > > OK? Ok ccardenas@ > > martijn@ > > Index: dhcp.c > === > RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v > retrieving revision 1.4 > diff -u -p -r1.4 dhcp.c > --- dhcp.c5 Nov 2017 20:01:09 - 1.4 > +++ dhcp.c15 Aug 2018 13:41:24 - > @@ -189,11 +189,6 @@ dhcp_request(struct vionet_dev *dev, cha > o += sizeof(server_addr); > } > > - resp.options[o++] = DHO_DOMAIN_NAME_SERVERS; > - resp.options[o++] = sizeof(server_addr); > - memcpy([o], _addr, sizeof(server_addr)); > - o += sizeof(server_addr); > - > resp.options[o++] = DHO_SUBNET_MASK; > resp.options[o++] = sizeof(mask); > mask.s_addr = htonl(0xfffe); >
Re: LACP Administrative Knobs
On Fri, Aug 10, 2018 at 11:36:01AM +0200, Remi Locherer wrote: > On 2018-08-09 03:53, Carlos Cardenas wrote: > > On Mon, Aug 06, 2018 at 08:18:23PM -0700, Carlos Cardenas wrote: > > > Howdy. > > > > > > Attached is a patch from my work that started at g2k18 on adding > > > administrative knobs to our LACP driver. > > > > > > The driver now has a new ioctl (SIOCxTRUNKOPTS), which for now only > > > has options for LACP: > > > * Mode - Active or Passive (default Active) > > > * Timeout - Fast or Slow (default Slow) > > > * System Priority - 1(high) to 65535(low) (default 32768) > > > * Port Priority - 1(high) to 65535(low) (default 32768) > > > * IFQ Priority - 0 to NUM_QUEUES (default 6) > > > > > > At the moment, ifconfig only has options for lacpmode and lacptimeout > > > plumbed as those are the immediate need. > > > > > > The approach taken for the options was to make them on a "trunk" vs a > > > "port" as what's typically seen on various NOSes (JunOS, NXOS, etc...) > > > as it's uncommon for a host to have one link "Passive" and the other > > > "Active" in a given trunk. > > > > > > Just like on a NOS, when applying lacpmode or lacptimeout, the > > > settings > > > are immediately applied to all existing ports in the trunk and to all > > > future ports brought into the trunk. > > > > > > When using lacpmode/lacptimeout, they can be used on the same line > > > creating the trunk. > > > > > > Here's an example hostname.trunk0: > > > trunkproto lacp lacptimeout fast > > > trunkport em0 > > > trunkport em1 > > > dhcp > > > inet6 autoconf > > > > > > Testing done: > > > Arch - amd64 > > > - Kernel: bsd.rd and GENERIC.MP > > > - NIC Driver: em and oce > > > - NOS: JunOS 18.1 > > > - MLAG-SETUP: No > > > > > > I would like to solicit testers with additional NIC drivers, NOSes, > > > and > > > MLAG setups to ensure there's no regressions. > > > > > > Comments? Ok? > > > > I've updated the ifconfig piece to be a bit cleaner (prompted by > > deraadt). > > Hi Carlos, > > this works fine in my test. > > I used an APU1 (amd64) with re and a Ruckus ICX7150 (FI 08.0.80 switching). > With > active debug on the switch it's immediately visible when the timeout or mode > are > changed on the OpenBSD side: > > LACP RX(port=1/1/3,T=13774) -> PDU info > AC SP32768.000d.b935.3b41:PP32768.PN 3=K16 > (Pas/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > ^^ > PA SP1.903a.722a.2370:PP1.PN 3=K20 (Act/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > LACP RX(port=1/1/4,T=13774) -> PDU info > AC SP32768.000d.b935.3b41:PP32768.PN 2=K16 > (Pas/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > PA SP1.903a.722a.2370:PP1.PN 4=K20 (Act/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > LACP RX(port=1/1/3,T=14010) -> PDU info > AC SP32768.000d.b935.3b41:PP32768.PN 3=K16 > (Act/ST/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > ^^ > PA SP1.903a.722a.2370:PP1.PN 3=K20 (Act/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > LACP RX(port=1/1/4,T=14010) -> PDU info > AC SP32768.000d.b935.3b41:PP32768.PN 2=K16 > (Act/ST/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > PA SP1.903a.722a.2370:PP1.PN 4=K20 (Act/LT/Agg/SYN/Col/Dist/UN-Def/UN-Exp) > > Small nit: > In man trunk I would prefer when the defaults are listed in the lacp section > and > not in the caveats section. Your diff addresses the caveat ;-). > > OK remi > Cool...I'll rework the manpage to reflect that before commit. +--+ Carlos
Re: LACP Administrative Knobs
On Mon, Aug 06, 2018 at 08:18:23PM -0700, Carlos Cardenas wrote:
> Howdy.
>
> Attached is a patch from my work that started at g2k18 on adding
> administrative knobs to our LACP driver.
>
> The driver now has a new ioctl (SIOCxTRUNKOPTS), which for now only
> has options for LACP:
> * Mode - Active or Passive (default Active)
> * Timeout - Fast or Slow (default Slow)
> * System Priority - 1(high) to 65535(low) (default 32768)
> * Port Priority - 1(high) to 65535(low) (default 32768)
> * IFQ Priority - 0 to NUM_QUEUES (default 6)
>
> At the moment, ifconfig only has options for lacpmode and lacptimeout
> plumbed as those are the immediate need.
>
> The approach taken for the options was to make them on a "trunk" vs a
> "port" as what's typically seen on various NOSes (JunOS, NXOS, etc...)
> as it's uncommon for a host to have one link "Passive" and the other
> "Active" in a given trunk.
>
> Just like on a NOS, when applying lacpmode or lacptimeout, the settings
> are immediately applied to all existing ports in the trunk and to all
> future ports brought into the trunk.
>
> When using lacpmode/lacptimeout, they can be used on the same line
> creating the trunk.
>
> Here's an example hostname.trunk0:
> trunkproto lacp lacptimeout fast
> trunkport em0
> trunkport em1
> dhcp
> inet6 autoconf
>
> Testing done:
> Arch - amd64
> - Kernel: bsd.rd and GENERIC.MP
> - NIC Driver: em and oce
> - NOS: JunOS 18.1
> - MLAG-SETUP: No
>
> I would like to solicit testers with additional NIC drivers, NOSes, and
> MLAG setups to ensure there's no regressions.
>
> Comments? Ok?
I've updated the ifconfig piece to be a bit cleaner (prompted by
deraadt).
+--+
Carlos
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /home/los/cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.312
diff -u -p -r1.312 ifconfig.8
--- sbin/ifconfig/ifconfig.819 Jul 2018 19:16:36 - 1.312
+++ sbin/ifconfig/ifconfig.88 Aug 2018 17:42:56 -
@@ -1627,6 +1627,16 @@ Set the trunk protocol.
Refer to
.Xr trunk 4
for a complete list of the available protocols.
+.It Cm lacpmode Ar mode
+Set the LACP trunk mode to either
+.Ar active
+or
+.Ar passive .
+.It Cm lacptimeout Ar speed
+Set the LACP timeout speed to either
+.Ar fast
+or
+.Ar slow .
.El
.Sh TUNNEL
.nr nS 1
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /home/los/cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.369
diff -u -p -r1.369 ifconfig.c
--- sbin/ifconfig/ifconfig.c13 Jul 2018 08:41:32 - 1.369
+++ sbin/ifconfig/ifconfig.c8 Aug 2018 17:42:56 -
@@ -170,7 +170,12 @@ intshownet80211chans;
intshownet80211nodes;
intshowclasses;
-struct ifencap;
+struct ifencap;
+
+const char *lacpmodeactive = "active";
+const char *lacpmodepassive = "passive";
+const char *lacptimeoutfast = "fast";
+const char *lacptimeoutslow = "slow";
void notealias(const char *, int);
void setifaddr(const char *, int);
@@ -252,6 +257,8 @@ voidsetautoconf(const char *, int);
void settrunkport(const char *, int);
void unsettrunkport(const char *, int);
void settrunkproto(const char *, int);
+void settrunklacpmode(const char *, int);
+void settrunklacptimeout(const char *, int);
void trunk_status(void);
void list_cloners(void);
@@ -408,6 +415,8 @@ const structcmd {
{ "trunkport", NEXTARG,0, settrunkport },
{ "-trunkport", NEXTARG,0, unsettrunkport },
{ "trunkproto", NEXTARG,0, settrunkproto },
+ { "lacpmode", NEXTARG,0, settrunklacpmode },
+ { "lacptimeout", NEXTARG, 0, settrunklacptimeout },
{ "anycast",IN6_IFF_ANYCAST,0, setia6flags },
{ "-anycast", -IN6_IFF_ANYCAST, 0, setia6flags },
{ "tentative", IN6_IFF_TENTATIVE, 0, setia6flags },
@@ -3990,6 +3999,72 @@ settrunkproto(const char *val, int d)
strlcpy(ra.ra_ifname, name, sizeof(ra.ra_ifname));
if (ioctl(s, SIOCSTRUNK, ) != 0)
err(1, "SIOCSTRUNK");
+}
+
+void
+settrunklacpmode(const char *val, int d)
+{
+ struct trunk_reqall ra;
+ struct trunk_opts tops;
+
+ bzero(, sizeof(ra));
+ strlcpy(ra.ra_ifname, name, sizeof(ra.ra_ifname));
+
+ if (ioctl(s, SIOCGTRUNK, ) != 0)
+ err(1, "SIOCGTRUNK");
+
+ if (ra.ra_proto != TRUNK_PROTO_LACP)
+ errx(1, "Invalid option for trunk: %s", name);
+
+ if (strcmp(v
LACP Administrative Knobs
Howdy.
Attached is a patch from my work that started at g2k18 on adding
administrative knobs to our LACP driver.
The driver now has a new ioctl (SIOCxTRUNKOPTS), which for now only
has options for LACP:
* Mode - Active or Passive (default Active)
* Timeout - Fast or Slow (default Slow)
* System Priority - 1(high) to 65535(low) (default 32768)
* Port Priority - 1(high) to 65535(low) (default 32768)
* IFQ Priority - 0 to NUM_QUEUES (default 6)
At the moment, ifconfig only has options for lacpmode and lacptimeout
plumbed as those are the immediate need.
The approach taken for the options was to make them on a "trunk" vs a
"port" as what's typically seen on various NOSes (JunOS, NXOS, etc...)
as it's uncommon for a host to have one link "Passive" and the other
"Active" in a given trunk.
Just like on a NOS, when applying lacpmode or lacptimeout, the settings
are immediately applied to all existing ports in the trunk and to all
future ports brought into the trunk.
When using lacpmode/lacptimeout, they can be used on the same line
creating the trunk.
Here's an example hostname.trunk0:
trunkproto lacp lacptimeout fast
trunkport em0
trunkport em1
dhcp
inet6 autoconf
Testing done:
Arch - amd64
- Kernel: bsd.rd and GENERIC.MP
- NIC Driver: em and oce
- NOS: JunOS 18.1
- MLAG-SETUP: No
I would like to solicit testers with additional NIC drivers, NOSes, and
MLAG setups to ensure there's no regressions.
Comments? Ok?
+--+
Carlos
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /home/los/cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.312
diff -u -p -r1.312 ifconfig.8
--- sbin/ifconfig/ifconfig.819 Jul 2018 19:16:36 - 1.312
+++ sbin/ifconfig/ifconfig.86 Aug 2018 18:24:11 -
@@ -1627,6 +1627,16 @@ Set the trunk protocol.
Refer to
.Xr trunk 4
for a complete list of the available protocols.
+.It Cm lacpmode Ar mode
+Set the LACP trunk mode to either
+.Ar active
+or
+.Ar passive .
+.It Cm lacptimeout Ar speed
+Set the LACP timeout speed to either
+.Ar fast
+or
+.Ar slow .
.El
.Sh TUNNEL
.nr nS 1
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /home/los/cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.369
diff -u -p -r1.369 ifconfig.c
--- sbin/ifconfig/ifconfig.c13 Jul 2018 08:41:32 - 1.369
+++ sbin/ifconfig/ifconfig.c6 Aug 2018 18:24:11 -
@@ -170,7 +170,12 @@ intshownet80211chans;
intshownet80211nodes;
intshowclasses;
-struct ifencap;
+struct ifencap;
+
+const char *lacpmodeactive = "active";
+const char *lacpmodepassive = "passive";
+const char *lacptimeoutfast = "fast";
+const char *lacptimeoutslow = "slow";
void notealias(const char *, int);
void setifaddr(const char *, int);
@@ -252,6 +257,8 @@ voidsetautoconf(const char *, int);
void settrunkport(const char *, int);
void unsettrunkport(const char *, int);
void settrunkproto(const char *, int);
+void settrunklacpmode(const char *, int);
+void settrunklacptimeout(const char *, int);
void trunk_status(void);
void list_cloners(void);
@@ -408,6 +415,8 @@ const structcmd {
{ "trunkport", NEXTARG,0, settrunkport },
{ "-trunkport", NEXTARG,0, unsettrunkport },
{ "trunkproto", NEXTARG,0, settrunkproto },
+ { "lacpmode", NEXTARG,0, settrunklacpmode },
+ { "lacptimeout", NEXTARG, 0, settrunklacptimeout },
{ "anycast",IN6_IFF_ANYCAST,0, setia6flags },
{ "-anycast", -IN6_IFF_ANYCAST, 0, setia6flags },
{ "tentative", IN6_IFF_TENTATIVE, 0, setia6flags },
@@ -3990,6 +3999,77 @@ settrunkproto(const char *val, int d)
strlcpy(ra.ra_ifname, name, sizeof(ra.ra_ifname));
if (ioctl(s, SIOCSTRUNK, ) != 0)
err(1, "SIOCSTRUNK");
+}
+
+void
+settrunklacpmode(const char *val, int d)
+{
+ struct trunk_reqall ra;
+ struct trunk_opts tops;
+
+ bzero(, sizeof(ra));
+ strlcpy(ra.ra_ifname, name, sizeof(ra.ra_ifname));
+
+ if (ioctl(s, SIOCGTRUNK, ) != 0)
+ err(1, "SIOCGTRUNK");
+
+ if (ra.ra_proto != TRUNK_PROTO_LACP) {
+ printf("proto: %d\n", ra.ra_proto);
+ errx(1, "Invalid option for trunk: %s", name);
+ } else {
+ if (strcmp(val, lacpmodeactive) != 0 &&
+ strcmp(val, lacpmodepassive) != 0) {
+ errx(1, "Invalid lacpmode option for trunk: %s", name);
+ }
+
+ bzero(, sizeof(tops));
+ strlcpy(tops.to_ifname, name, sizeof(tops.to_ifname));
+ tops.to_proto = TRUNK_PROTO_LACP;
+ tops.to_opts |= TRUNK_OPT_LACP_MODE;
+ if (strcmp(val, lacpmodeactive) == 0)
+
Re: monitor mode for iwm(4)
Looks good on a 7260.
ok ccardenas@
+--+
Carlos
On Wed, May 23, 2018 at 12:31:45PM +0200, Stefan Sperling wrote:
> This diff implements monitor mode for iwm(4).
>
> To use it, enable 11n mode, monitor mode, and configure a channel (e.g. 11):
>
> ifconfig iwm0 mode 11n mediaopt monitor
> ifconfig iwm0 chan 11
> ifconfig iwm0 up
>
> This command should now display wireless frames on the air:
>
> tcpdump -n -i iwm0 -y IEEE802_11_RADIO -v
>
> Record frames with tcpdump like this (pcap file can be viewed in wireshark):
>
> tcpdump -n -i iwm0 -s 4096 -y IEEE802_11_RADIO -w /tmp/iwm.pcap
>
> Note that 11n/11ac frames sent with >= 40 MHz channel width won't be
> captured. The hardware could do it but our stack is not passing the
> necessary configuration parameters to the driver, so the driver cannot
> configure the hardware appropriately (a similar problem as bwfm(4) has
> with our stack's incomplete 11n support).
> But the corresponding leading RTS and trailing BlockAck frames can be
> captured, which allows us to manually deduce the presence of such frames.
>
> Tested on a 7265 device.
>
> ok?
>
> Index: if_iwm.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
> retrieving revision 1.229
> diff -u -p -r1.229 if_iwm.c
> --- if_iwm.c 15 May 2018 19:48:23 - 1.229
> +++ if_iwm.c 23 May 2018 10:10:45 -
> @@ -426,7 +426,7 @@ uint8_t iwm_ridx2rate(struct ieee80211_r
> int iwm_rval2ridx(int);
> void iwm_ack_rates(struct iwm_softc *, struct iwm_node *, int *, int *);
> void iwm_mac_ctxt_cmd_common(struct iwm_softc *, struct iwm_node *,
> - struct iwm_mac_ctx_cmd *, uint32_t, int);
> + struct iwm_mac_ctx_cmd *, uint32_t);
> void iwm_mac_ctxt_cmd_fill_sta(struct iwm_softc *, struct iwm_node *,
> struct iwm_mac_data_sta *, int);
> int iwm_mac_ctxt_cmd(struct iwm_softc *, struct iwm_node *, uint32_t, int);
> @@ -4446,6 +4446,7 @@ void
> iwm_power_build_cmd(struct iwm_softc *sc, struct iwm_node *in,
> struct iwm_mac_power_cmd *cmd)
> {
> + struct ieee80211com *ic = >sc_ic;
> struct ieee80211_node *ni = >in_ni;
> int dtim_period, dtim_msec, keep_alive;
>
> @@ -4467,7 +4468,8 @@ iwm_power_build_cmd(struct iwm_softc *sc
> keep_alive = roundup(keep_alive, 1000) / 1000;
> cmd->keep_alive_seconds = htole16(keep_alive);
>
> - cmd->flags = htole16(IWM_POWER_FLAGS_POWER_SAVE_ENA_MSK);
> + if (ic->ic_opmode != IEEE80211_M_MONITOR)
> + cmd->flags = htole16(IWM_POWER_FLAGS_POWER_SAVE_ENA_MSK);
> }
>
> int
> @@ -4494,13 +4496,15 @@ iwm_power_mac_update_mode(struct iwm_sof
> int
> iwm_power_update_device(struct iwm_softc *sc)
> {
> - struct iwm_device_power_cmd cmd = {
> - .flags = htole16(IWM_DEVICE_POWER_FLAGS_POWER_SAVE_ENA_MSK),
> - };
> + struct iwm_device_power_cmd cmd = { };
> + struct ieee80211com *ic = >sc_ic;
>
> if (!(sc->sc_capaflags & IWM_UCODE_TLV_FLAGS_DEVICE_PS_CMD))
> return 0;
>
> + if (ic->ic_opmode != IEEE80211_M_MONITOR)
> + cmd.flags = htole16(IWM_DEVICE_POWER_FLAGS_POWER_SAVE_ENA_MSK);
> +
> return iwm_send_cmd_pdu(sc,
> IWM_POWER_TABLE_CMD, 0, sizeof(cmd), );
> }
> @@ -4562,7 +4566,12 @@ iwm_add_sta_cmd(struct iwm_softc *sc, st
> add_sta_cmd.tfd_queue_msk |=
> htole32(1 << iwm_ac_to_tx_fifo[ac]);
> }
> - IEEE80211_ADDR_COPY(_sta_cmd.addr, in->in_ni.ni_bssid);
> + if (ic->ic_opmode == IEEE80211_M_MONITOR)
> + IEEE80211_ADDR_COPY(_sta_cmd.addr,
> + etherbroadcastaddr);
> + else
> + IEEE80211_ADDR_COPY(_sta_cmd.addr,
> + in->in_ni.ni_bssid);
> }
> add_sta_cmd.add_modify = update ? 1 : 0;
> add_sta_cmd.station_flags_msk
> @@ -5230,7 +5239,7 @@ iwm_ack_rates(struct iwm_softc *sc, stru
>
> void
> iwm_mac_ctxt_cmd_common(struct iwm_softc *sc, struct iwm_node *in,
> -struct iwm_mac_ctx_cmd *cmd, uint32_t action, int assoc)
> +struct iwm_mac_ctx_cmd *cmd, uint32_t action)
> {
> #define IWM_EXP2(x) ((1 << (x)) - 1)/* CWmin = 2^ECWmin - 1 */
> struct ieee80211com *ic = >sc_ic;
> @@ -5242,12 +5251,21 @@ iwm_mac_ctxt_cmd_common(struct iwm_softc
> in->in_color));
> cmd->action = htole32(action);
>
> - cmd->mac_type = htole32(IWM_FW_MAC_TYPE_BSS_STA);
> + if (ic->ic_opmode == IEEE80211_M_MONITOR)
> + cmd->mac_type = htole32(IWM_FW_MAC_TYPE_LISTENER);
> + else if (ic->ic_opmode == IEEE80211_M_STA)
> + cmd->mac_type = htole32(IWM_FW_MAC_TYPE_BSS_STA);
> + else
> + panic("unsupported operating mode %d\n", ic->ic_opmode);
> cmd->tsf_id = htole32(IWM_TSF_ID_A);
>
> IEEE80211_ADDR_COPY(cmd->node_addr, ic->ic_myaddr);
Re: [vmd] errror -> error
On Wed, Apr 25, 2018 at 07:59:52PM +0200, llgx...@gmail.com wrote:
> Hi,
>
> I spotted a couple more typos:
Thanks, I'll commit this later this evening.
+--+
Carlos
>
> Index: vioscsi.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vioscsi.c,v
> retrieving revision 1.4
> diff -u -p -u -p -r1.4 vioscsi.c
> --- vioscsi.c 19 Jan 2018 14:23:52 - 1.4
> +++ vioscsi.c 25 Apr 2018 17:49:33 -
> @@ -203,7 +203,7 @@ vioscsi_start_read(struct vioscsi_dev *d
>
> nomem:
> free(info);
> - log_warn("malloc errror vioscsi read");
> + log_warn("malloc error vioscsi read");
> return (NULL);
> }
>
> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.56
> diff -u -p -u -p -r1.56 virtio.c
> --- virtio.c 1 Feb 2018 18:33:27 - 1.56
> +++ virtio.c 25 Apr 2018 17:49:33 -
> @@ -363,7 +363,7 @@ vioblk_start_read(struct vioblk_dev *dev
>
> nomem:
> free(info);
> - log_warn("malloc errror vioblk read");
> + log_warn("malloc error vioblk read");
> return (NULL);
> }
>
> @@ -404,7 +404,7 @@ vioblk_start_write(struct vioblk_dev *de
>
> nomem:
> free(info);
> - log_warn("malloc errror vioblk write");
> + log_warn("malloc error vioblk write");
> return (NULL);
> }
>
>
> Thank you
>
Add AMD 15h/3xh and Kaveri to pcidevs
Howdy. This patch adds AMD 15h/3xh (i.e. A8-7670K) and Kaveri devices. dmesg and patch attached. Comments. Ok? +--+ Carlos OpenBSD 6.3-current (GENERIC.MP) #1: Thu Apr 19 11:04:10 PDT 2018 los@rollo.castle:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 16046288896 (15302MB) avail mem = 15552061440 (14831MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xecc20 (55 entries) bios0: vendor American Megatrends Inc. version "V1.3" date 04/13/2016 bios0: MSI MS-7969 acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP APIC FPDT FIDT MCFG HPET UEFI IVRS SSDT SSDT CRAT SSDT SSDT acpi0: wakeup devices SBAZ(S4) P0PC(S4) OHC1(S4) EHC1(S4) OHC2(S4) EHC2(S4) OHC3(S4) EHC3(S4) OHC4(S4) XHC0(S4) XHC1(S4) PE20(S4) PE21(S4) PE23(S4) PB21(S4) PB22(S4) [...] acpitimer0 at acpi0: 3579545 Hz, 32 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 16 (boot processor) cpu0: AMD A8-7670K Radeon R7, 10 Compute Cores 4C+6G, 3595.94 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,DBKP,PERFTSC,ITSC,FSGSBASE,BMI1 cpu0: 96KB 64b/line 3-way I-cache, 16KB 64b/line 4-way D-cache, 2MB 64b/line 16-way L2 cache cpu0: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, IBE cpu1 at mainbus0: apid 17 (application processor) cpu1: AMD A8-7670K Radeon R7, 10 Compute Cores 4C+6G, 3593.29 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,DBKP,PERFTSC,ITSC,FSGSBASE,BMI1 cpu1: 96KB 64b/line 3-way I-cache, 16KB 64b/line 4-way D-cache, 2MB 64b/line 16-way L2 cache cpu1: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 18 (application processor) cpu2: AMD A8-7670K Radeon R7, 10 Compute Cores 4C+6G, 3593.29 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,DBKP,PERFTSC,ITSC,FSGSBASE,BMI1 cpu2: 96KB 64b/line 3-way I-cache, 16KB 64b/line 4-way D-cache, 2MB 64b/line 16-way L2 cache cpu2: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu2: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 19 (application processor) cpu3: AMD A8-7670K Radeon R7, 10 Compute Cores 4C+6G, 3593.29 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,DBKP,PERFTSC,ITSC,FSGSBASE,BMI1 cpu3: 96KB 64b/line 3-way I-cache, 16KB 64b/line 4-way D-cache, 2MB 64b/line 16-way L2 cache cpu3: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu3: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu3: smt 0, core 3, package 0 ioapic0 at mainbus0: apid 0 pa 0xfec0, version 21, 24 pins ioapic1 at mainbus0: apid 1 pa 0xfec01000, version 21, 32 pins acpimcfg0 at acpi0 addr 0xe000, bus 0-255 acpihpet0 at acpi0: 14318180 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 4 (P0PC) acpiprt2 at acpi0: bus -1 (PE20) acpiprt3 at acpi0: bus -1 (PE21) acpiprt4 at acpi0: bus -1 (PE23) acpiprt5 at acpi0: bus 1 (PB21) acpiprt6 at acpi0: bus -1 (PB22) acpiprt7 at acpi0: bus 2 (PB31) acpiprt8 at acpi0: bus -1 (PB32) acpiprt9 at acpi0: bus 3 (PB33) acpiprt10 at acpi0: bus -1 (PB34) acpiprt11 at acpi0: bus -1 (PE22) acpicpu0 at acpi0: C2(0@400 io@0x1771), C1(@1 halt!), PSS acpicpu1 at acpi0: C2(0@400 io@0x1771), C1(@1 halt!), PSS acpicpu2 at acpi0: C2(0@400 io@0x1771), C1(@1 halt!), PSS acpicpu3 at acpi0: C2(0@400 io@0x1771), C1(@1 halt!), PSS acpicmos0 at acpi0 acpibtn0 at acpi0: PWRB "PNP0C14" at acpi0 not configured
Re: vmd(4) close vmm parent socket
On Fri, Apr 13, 2018 at 11:29:06AM +0200, Martijn van Duren wrote:
> Hello tech@,
>
> Playing with vmd I noticed that a vm process has vmm's socket to the
> parent process still open.
>
> Patch below works for me.
>
> OK?
>
> martijn@
Nice find.
Ok ccardenas@
+--+
Carlos
>
> Index: vmm.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 vmm.c
> --- vmm.c 5 Feb 2018 05:01:08 - 1.80
> +++ vmm.c 13 Apr 2018 09:27:14 -
> @@ -664,6 +664,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t
> } else {
> /* Child */
> close(fds[0]);
> + close(PROC_PARENT_SOCK_FILENO);
>
> ret = start_vm(vm, fds[1]);
>
>
Re: vmd(8) sends global config too late
On Fri, Mar 09, 2018 at 07:06:44PM +0100, Martijn van Duren wrote:
> Hello Carlos,
>
> On 02/22/18 03:30, Carlos Cardenas wrote:
> > On Wed, Feb 21, 2018 at 11:27:05PM +0100, Martijn van Duren wrote:
> >> Hello tech@,
> >>
> >> When playing around with local prefix in vm.conf(5) I noticed that it
> >> wasn't picked up by my vm. The reason for this was that config_setvm was
> >> called before config_setconfig. Diff below fixes this.
> >>
> >> OK?
> >>
> >> martijn@
> >
> > Howdy.
> >
> > I'm confused by the problem statement and use case(s) you are referring
> > to so let me rephrase what I think you might be saying:
> > * when a custom prefix is defined in vm.conf, local interfaces doesn't
> > work on vms that are not disabled by default
> >
> > Is that correct?
>
> Yes
> >
> > When I put a custom prefix in vm.conf and:
> > * launch vms via vmctl
> > or
> > * start vms that do not start up automatically
> > the custom prefix is used.
>
> Also correct
> >
> > My comments on this patch are:
> > * the calling of config_setconfig is called too early; it needs to
> > happen after the "env->vmd_noaction" check
>
> Fixed
> > * if we had an issue with prefix being sent too late on vmd startup,
> > then we'll also have an issue in vmd_reload as well. The
> > config_setconfig call needs to happen after we parse the new config
> > file.
>
> Fixed
> >
> > For testing, can you ensure the following cases work as designed:
> > 1) launching vms via vmctl
> > 2) start vms that do not start automatically
> > 3) vms that do start automatically
> > 4) Modify prefix in vm.conf, `vmctl reload`, and ensure the above three
> > items work
> >with the new prefix.
>
> Done
> >
> > Thanks.
> >
> > +--+
> > Carlos
This patch looks good.
Thanks again Martijn.
OK ccardenas@
+--+
Carlos
>
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 vmd.c
> --- vmd.c 10 Jan 2018 14:59:59 - 1.79
> +++ vmd.c 9 Mar 2018 18:06:15 -
> @@ -823,6 +823,10 @@ vmd_configure(void)
> exit(0);
> }
>
> + /* Send shared global configuration to all children */
> + if (config_setconfig(env) == -1)
> + return (-1);
> +
> TAILQ_FOREACH(vsw, env->vmd_switches, sw_entry) {
> if (vsw->sw_running)
> continue;
> @@ -845,10 +849,6 @@ vmd_configure(void)
> return (-1);
> }
>
> - /* Send shared global configuration to all children */
> - if (config_setconfig(env) == -1)
> - return (-1);
> -
> return (0);
> }
>
> @@ -888,16 +888,18 @@ vmd_reload(unsigned int reset, const cha
> vm_remove(vm);
> }
> }
> -
> - /* Update shared global configuration in all children */
> - if (config_setconfig(env) == -1)
> - return (-1);
> }
>
> if (parse_config(filename) == -1) {
> log_debug("%s: failed to load config file %s",
> __func__, filename);
> return (-1);
> + }
> +
> + if (reload) {
> + /* Update shared global configuration in all children */
> + if (config_setconfig(env) == -1)
> + return (-1);
> }
>
> TAILQ_FOREACH(vsw, env->vmd_switches, sw_entry) {
Re: update pcidevs for thinkpad e475 devices
On Mon, Feb 26, 2018 at 03:25:26PM +1100, Jonathan Gray wrote: > On Sun, Feb 25, 2018 at 06:27:11PM -0800, Carlos Cardenas wrote: > > Howdy. > > > > Attached is a patch for pcidevs for thinkpad e475 with an > > AMD A10-9600P R5 (Carrizo) along with the dmesg output. > > Items added: > > * O2 Micro SD/MMC > > * various AMD 15h/6xh devs > > * Carrizo video > > A10-9600P is 'Bristol Ridge' which seems to be a tweaked Carrizo design. > Most of the devices mentioned would have first appeared in Carrizo > so it is right to use that name instead. > > And just to confuse things Carrizo-L is a quite different family 16h apu... My head's about to implode with trying to keep those code names straight ;-) Attached is an updated patch with your suggestions. +--+ Carlos Index: pcidevs === RCS file: /home/los/cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.1836 diff -u -p -r1.1836 pcidevs --- pcidevs 23 Feb 2018 07:04:57 - 1.1836 +++ pcidevs 26 Feb 2018 05:22:44 - @@ -395,6 +395,7 @@ product O2MICRO OZ7134 0x7134 OZ711MP1 product O2MICRO OZ7135 0x7135 OZ711EZ1 CardBus product O2MICRO OZ7136 0x7136 OZ711SP1 CardBus product O2MICRO OZ7223 0x7223 OZ711E0 CardBus +product O2MICRO OZ8621 0x8621 0Z8621 SD/MMC /* 3Com Products */ product 3COM 3C985 0x0001 3c985 @@ -732,6 +733,19 @@ product AMD AMD64_16_HB0x1536 AMD64 16 product AMD CCP0x1537 CCP product AMD AMD64_16_3X_RC 0x1566 AMD64 16h Root Complex product AMD AMD64_16_3X_HB 0x156b AMD64 16h Host +product AMD AMD64_15_6X_LINK 0x1570 AMD64 15h Link Cfg +product AMD AMD64_15_6X_ADDR 0x1571 AMD64 15h Address Map +product AMD AMD64_15_6X_DRAM 0x1572 AMD64 15h DRAM Cfg +product AMD AMD64_15_6X_MISC 0x1573 AMD64 15h Misc Cfg +product AMD AMD64_15_6X_CPU_PM 0x1574 AMD64 15h CPU Power +product AMD AMD64_15_6X_MISC_2 0x1575 AMD64 15h Misc Cfg +product AMD AMD64_15_6X_RC 0x1576 AMD64 15h Root Complex +product AMD AMD64_15_6X_IOMMU 0x1577 AMD64 15h IOMMU +product AMD AMD64_15_6X_PSP0x1578 AMD64 15h PSP 2.0 +product AMD AMD64_15_6X_AUDIO 0x157a AMD64 15h HD Audio +product AMD AMD64_15_6X_HB 0x157b AMD64 15h Host +product AMD AMD64_15_6X_PCIE_1 0x157c AMD64 15h PCIE +product AMD AMD64_15_6X_HB_2 0x157d AMD64 15h Host product AMD AMD64_16_3X_LINK 0x1580 AMD64 16h Link Cfg product AMD AMD64_16_3X_ADDR 0x1581 AMD64 16h Address Map product AMD AMD64_16_3X_DRAM 0x1582 AMD64 16h DRAM Cfg @@ -843,6 +857,15 @@ product AMD HUDSON2_PCI0x780f Hudson-2 product AMD HUDSON2_XHCI 0x7812 Hudson-2 xHCI product AMD BOLTON_SDMMC 0x7813 Bolton SD/MMC product AMD BOLTON_XHCI0x7814 Bolton xHCI +product AMD CARRIZO_SATA_1 0x7900 Carrizo SATA +product AMD CARRIZO_SATA_2 0x7901 Carrizo AHCI +product AMD CARRIZO_SATA_3 0x7902 Carrizo RAID +product AMD CARRIZO_SATA_4 0x7903 Carrizo RAID +product AMD CARRIZO_SATA_5 0x7904 Carrizo AHCI +product AMD CARRIZO_EHCI 0x7908 Carrizo USB2 +product AMD CARRIZO_SMB0x790b Carrizo SMBus +product AMD CARRIZO_LPC0x790e Carrizo LPC +product AMD CARRIZO_XHCI 0x7914 Carrizo xHCI product AMD RS780_HB 0x9600 RS780 Host product AMD RS880_HB 0x9601 RS880 Host product AMD RS780_PCIE_1 0x9602 RS780 PCIE @@ -1782,6 +1805,11 @@ product ATI RADEON_HD73100x9809 Radeon product ATI RADEON_HD7290 0x980a Radeon HD 7290 product ATI RADEON_HDA 0x9840 Radeon HD Audio product ATI MULLINS_1 0x9854 Mullins +product ATI CARRIZO_1 0x9870 Carrizo +product ATI CARRIZO_2 0x9874 Carrizo +product ATI CARRIZO_3 0x9875 Carrizo +product ATI CARRIZO_4 0x9876 Carrizo +product ATI CARRIZO_5 0x9877 Carrizo product ATI ARUBA_10x9900 Aruba product ATI RADEON_HD7660D 0x9901 Radeon HD 7660D product ATI RADEON_HD7640G_1 0x9903 Radeon HD 7640G
vmctl: clarify console error message
Attached patch clears up an ambiguous error message when attaching to a console fails. The vm id is not guaranteed to be populated. Comments? Ok? +--+ Carlos Index: vmctl.c === RCS file: /home/los/cvs/src/usr.sbin/vmctl/vmctl.c,v retrieving revision 1.45 diff -u -p -r1.45 vmctl.c --- vmctl.c 3 Jan 2018 05:39:56 - 1.45 +++ vmctl.c 20 Feb 2018 05:09:09 - @@ -702,7 +702,7 @@ vm_console(struct vmop_info_result *list } } - errx(1, "console %d not found", info_id); + errx(1, "console not found"); } /*
update pcidevs for thinkpad e475 devices
Howdy. Attached is a patch for pcidevs for thinkpad e475 with an AMD A10-9600P R5 (Carrizo) along with the dmesg output. Items added: * O2 Micro SD/MMC * various AMD 15h/6xh devs * Carrizo video Comments? Ok? +--+ Carlos OpenBSD 6.2-current (GENERIC.MP) #0: Sun Feb 25 15:32:13 PST 2018 los@bjorn.castle:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 16524206080 (15758MB) avail mem = 16016396288 (15274MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xdb225000 (58 entries) bios0: vendor LENOVO version "R0EET44W (1.18 )" date 11/17/2017 bios0: LENOVO 20H4CTO1WW acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP UEFI HPET APIC MCFG SBST MSDM BATB SSDT IVRS CRAT TPM2 SSDT SSDT SSDT SSDT FPDT SSDT UEFI acpi0: wakeup devices GPP0(S4) GPP1(S4) GPP2(S4) GPP3(S4) GPP4(S4) GFX0(S4) GFX1(S4) GFX2(S4) GFX3(S4) GFX4(S4) XHC0(S3) EHC1(S3) SBAZ(S4) LID_(S3) SLPB(S3) acpitimer0 at acpi0: 3579545 Hz, 32 bits acpihpet0 at acpi0: 14318180 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 16 (boot processor) cpu0: AMD A10-9600P RADEON R5, 10 COMPUTE CORES 4C+6G, 2395.80 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,CPCTR,DBKP,PERFTSC,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2 cpu0: 96KB 64b/line 3-way I-cache, 32KB 64b/line 8-way D-cache, 1MB 64b/line 16-way L2 cache cpu0: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative acpihpet0: recalibrated TSC frequency 2395505539 Hz cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, IBE cpu1 at mainbus0: apid 17 (application processor) cpu1: AMD A10-9600P RADEON R5, 10 COMPUTE CORES 4C+6G, 2395.51 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,CPCTR,DBKP,PERFTSC,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2 cpu1: 96KB 64b/line 3-way I-cache, 32KB 64b/line 8-way D-cache, 1MB 64b/line 16-way L2 cache cpu1: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 18 (application processor) cpu2: AMD A10-9600P RADEON R5, 10 COMPUTE CORES 4C+6G, 2395.51 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,CPCTR,DBKP,PERFTSC,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2 cpu2: 96KB 64b/line 3-way I-cache, 32KB 64b/line 8-way D-cache, 1MB 64b/line 16-way L2 cache cpu2: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu2: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu2: smt 0, core 2, package 0 cpu3 at mainbus0: apid 19 (application processor) cpu3: AMD A10-9600P RADEON R5, 10 COMPUTE CORES 4C+6G, 2395.51 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,CPCTR,DBKP,PERFTSC,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2 cpu3: 96KB 64b/line 3-way I-cache, 32KB 64b/line 8-way D-cache, 1MB 64b/line 16-way L2 cache cpu3: ITLB 48 4KB entries fully associative, 24 4MB entries fully associative cpu3: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative cpu3: smt 0, core 3, package 0 ioapic0 at mainbus0: apid 4 pa 0xfec0, version 21, 24 pins , remapped to apid 4 ioapic1 at mainbus0: apid 5 pa 0xfec01000, version 21, 32 pins , remapped to apid 5 acpimcfg0 at acpi0 addr 0xf800, bus 0-63 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (GPP0) acpiprt2 at acpi0: bus 1 (GPP1) acpiprt3 at acpi0: bus 3 (GPP2) acpiprt4 at acpi0: bus 4 (GPP3) acpiprt5 at acpi0: bus -1 (GPP4) acpiprt6 at acpi0: bus -1 (GFX0) acpiprt7 at acpi0: bus -1 (GFX1) acpiprt8 at acpi0: bus -1 (GFX2) acpiprt9 at acpi0: bus -1 (GFX3) acpiprt10 at acpi0: bus -1 (GFX4) acpiec0 at acpi0 acpicpu0 at
Re: vmd(8) sends global config too late
On Wed, Feb 21, 2018 at 11:27:05PM +0100, Martijn van Duren wrote:
> Hello tech@,
>
> When playing around with local prefix in vm.conf(5) I noticed that it
> wasn't picked up by my vm. The reason for this was that config_setvm was
> called before config_setconfig. Diff below fixes this.
>
> OK?
>
> martijn@
Howdy.
I'm confused by the problem statement and use case(s) you are referring
to so let me rephrase what I think you might be saying:
* when a custom prefix is defined in vm.conf, local interfaces doesn't
work on vms that are not disabled by default
Is that correct?
When I put a custom prefix in vm.conf and:
* launch vms via vmctl
or
* start vms that do not start up automatically
the custom prefix is used.
My comments on this patch are:
* the calling of config_setconfig is called too early; it needs to
happen after the "env->vmd_noaction" check
* if we had an issue with prefix being sent too late on vmd startup,
then we'll also have an issue in vmd_reload as well. The
config_setconfig call needs to happen after we parse the new config
file.
For testing, can you ensure the following cases work as designed:
1) launching vms via vmctl
2) start vms that do not start automatically
3) vms that do start automatically
4) Modify prefix in vm.conf, `vmctl reload`, and ensure the above three items
work
with the new prefix.
Thanks.
+--+
Carlos
>
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 vmd.c
> --- vmd.c 18 Feb 2018 01:00:25 - 1.80
> +++ vmd.c 21 Feb 2018 22:26:35 -
> @@ -821,6 +821,10 @@ vmd_configure(void)
> exit(1);
> }
>
> + /* Send shared global configuration to all children */
> + if (config_setconfig(env) == -1)
> + return (-1);
> +
> if (env->vmd_noaction) {
> fprintf(stderr, "configuration OK\n");
> proc_kill(>vmd_ps);
> @@ -848,10 +852,6 @@ vmd_configure(void)
> if (config_setvm(>vmd_ps, vm, -1, vm->vm_params.vmc_uid)
> == -1)
> return (-1);
> }
> -
> - /* Send shared global configuration to all children */
> - if (config_setconfig(env) == -1)
> - return (-1);
>
> return (0);
> }
>
VMD: Fix Failure Start
Howdy.
Attached is a patch to fix the following condition:
When attempting to start a VM from vm.conf that fails (can't allocate
resources, etc...), do not remove vm from vm list.
Reported to bugs@ by mpi@.
Comments? Ok?
+--+
Carlos
Index: config.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.40
diff -u -p -a -u -r1.40 config.c
--- config.c5 Jan 2018 13:34:52 - 1.40
+++ config.c25 Jan 2018 03:25:23 -
@@ -416,8 +416,13 @@ config_setvm(struct privsep *ps, struct
free(tapfds);
}
- log_debug("%s: calling vm_remove", __func__);
- vm_remove(vm);
+ if (vm->vm_from_config) {
+ log_debug("%s: calling stop vm %d", __func__, vm->vm_vmid);
+ vm_stop(vm, 0);
+ } else {
+ log_debug("%s: calling remove vm %d", __func__, vm->vm_vmid);
+ vm_remove(vm);
+ }
errno = saved_errno;
if (errno == 0)
errno = EINVAL;
Re: VMD: vioscsi - fix large iso support
On Fri, Jan 19, 2018 at 07:35:08PM +1000, David Gwynne wrote:
>
>
> > On 19 Jan 2018, at 4:32 pm, Carlos Cardenas <cardena...@gmail.com> wrote:
> >
> > Howdy.
> >
> > Attached is a patch to fix handling images > 4GB support in Linux. I
> > confused image size vs blocks to determine when to send UINT32_MAX
> > and trigger a READ_CAPACITY_16.
> >
> > Identified by mlarkin@
> >
> > Comments? Ok?
>
> ok.
>
> after this you should make another fix though: READ CAPACITY 10 and 16 should
> return the last addressable block, not than the number of blocks.
>
> in other words you need to take 1 from n_blocks before processing it for
> scsi. a good exapmle of this is
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c#rev1.55 and
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/nvme.c.diff?r1=1.54=1.55
>
> dlg
That's already handled in READ_CAPACITY_* and GET_CONFIGURATION,
i.e. _lto4b(dev->n_blocks - 1, r_cap_data->length);
Along with checks in READ_* to ensure the requested lba is < n_blocks - 1.
Yes...I was bitten by that earlier on in the development of the driver. :-|
+--+
Carlos
>
> >
> > +--+
> > Carlos
> > Index: vioscsi.c
> > ===
> > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
> > retrieving revision 1.3
> > diff -u -p -a -u -r1.3 vioscsi.c
> > --- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
> > +++ vioscsi.c 19 Jan 2018 06:26:14 -
> > @@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
> > __func__, dev->sz, dev->n_blocks);
> >
> > /*
> > -* determine if size of iso image > UINT32_MAX
> > +* determine if num blocks of iso image > UINT32_MAX
> > * if it is, set addr to UINT32_MAX (0x)
> > * indicating to hosts that READ_CAPACITY_16 should
> > * be called to retrieve the full size
> > */
> > - if (dev->sz >= UINT32_MAX) {
> > + if (dev->n_blocks >= UINT32_MAX) {
> > _lto4b(UINT32_MAX, r_cap_data->addr);
> > _lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
> > log_warnx("%s: ISO sz %lld is bigger than "
> > @@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
> > config_random_read_desc->feature_code);
> > config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
> > config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
> > - if (dev->sz >= UINT32_MAX)
> > + if (dev->n_blocks >= UINT32_MAX)
> > _lto4b(UINT32_MAX, config_random_read_desc->block_size);
> > else
> > _lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
>
VMD: vioscsi - fix large iso support
Howdy.
Attached is a patch to fix handling images > 4GB support in Linux. I
confused image size vs blocks to determine when to send UINT32_MAX
and trigger a READ_CAPACITY_16.
Identified by mlarkin@
Comments? Ok?
+--+
Carlos
Index: vioscsi.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
retrieving revision 1.3
diff -u -p -a -u -r1.3 vioscsi.c
--- vioscsi.c 16 Jan 2018 06:10:45 - 1.3
+++ vioscsi.c 19 Jan 2018 06:26:14 -
@@ -637,12 +637,12 @@ vioscsi_handle_read_capacity(struct vios
__func__, dev->sz, dev->n_blocks);
/*
-* determine if size of iso image > UINT32_MAX
+* determine if num blocks of iso image > UINT32_MAX
* if it is, set addr to UINT32_MAX (0x)
* indicating to hosts that READ_CAPACITY_16 should
* be called to retrieve the full size
*/
- if (dev->sz >= UINT32_MAX) {
+ if (dev->n_blocks >= UINT32_MAX) {
_lto4b(UINT32_MAX, r_cap_data->addr);
_lto4b(VIOSCSI_BLOCK_SIZE_CDROM, r_cap_data->length);
log_warnx("%s: ISO sz %lld is bigger than "
@@ -1603,7 +1603,7 @@ vioscsi_handle_get_config(struct vioscsi
config_random_read_desc->feature_code);
config_random_read_desc->byte3 = CONFIG_RANDOM_READ_BYTE3;
config_random_read_desc->length = CONFIG_RANDOM_READ_LENGTH;
- if (dev->sz >= UINT32_MAX)
+ if (dev->n_blocks >= UINT32_MAX)
_lto4b(UINT32_MAX, config_random_read_desc->block_size);
else
_lto4b(dev->n_blocks - 1, config_random_read_desc->block_size);
VMD: vioscsi - add REPORT_LUNS opcode
Howdy.
Attached is a patch that includes handling the REPORT_LUNS opcode which
is used by SeaBIOS 1.11.x .
Comments? Ok?
+--+
Carlos
Index: vioscsi.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
retrieving revision 1.2
diff -u -p -a -u -r1.2 vioscsi.c
--- vioscsi.c 15 Jan 2018 04:26:58 - 1.2
+++ vioscsi.c 16 Jan 2018 04:48:03 -
@@ -772,6 +772,105 @@ read_capacity_16_out:
}
static int
+vioscsi_handle_report_luns(struct vioscsi_dev *dev,
+struct virtio_scsi_req_hdr *req, struct virtio_vq_acct *acct)
+{
+ int ret = 0;
+ struct virtio_scsi_res_hdr resp;
+ uint32_t rpl_length;
+ struct scsi_report_luns *rpl;
+ struct vioscsi_report_luns_data *reply_rpl;
+
+ memset(, 0, sizeof(resp));
+ rpl = (struct scsi_report_luns *)(req->cdb);
+ rpl_length = _4btol(rpl->length);
+
+ log_debug("%s: REPORT_LUNS Report 0x%x Length %d", __func__,
+ rpl->selectreport, rpl_length);
+
+ if (rpl_length < RPL_MIN_SIZE) {
+ log_debug("%s: RPL_Length %d < %d (RPL_MIN_SIZE)", __func__,
+ rpl_length, RPL_MIN_SIZE);
+
+ vioscsi_prepare_resp(,
+ VIRTIO_SCSI_S_OK, SCSI_CHECK, SKEY_ILLEGAL_REQUEST,
+ SENSE_ILLEGAL_CDB_FIELD, SENSE_DEFAULT_ASCQ);
+
+ /* Move index for response */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc,
+ acct->req_desc, &(acct->resp_idx));
+
+ if (write_mem(acct->resp_desc->addr, ,
+ acct->resp_desc->len)) {
+ log_warnx("%s: unable to set ERR "
+ "status data @ 0x%llx", __func__,
+ acct->resp_desc->addr);
+ } else {
+ ret = 1;
+ dev->cfg.isr_status = 1;
+ /* Move ring indexes */
+ vioscsi_next_ring_item(dev, acct->avail, acct->used,
+ acct->req_desc, acct->req_idx);
+ }
+ goto rpl_out;
+
+ }
+
+ reply_rpl = calloc(1, sizeof(*reply_rpl));
+
+ if (reply_rpl == NULL) {
+ log_warnx("%s: cannot alloc reply_rpl", __func__);
+ goto rpl_out;
+ }
+
+ _lto4b(RPL_SINGLE_LUN, reply_rpl->length);
+ memcpy(reply_rpl->lun, req->lun, RPL_SINGLE_LUN);
+
+ vioscsi_prepare_resp(,
+ VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);
+
+ /* Move index for response */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->req_desc,
+ &(acct->resp_idx));
+
+ dprintf("%s: writing resp to 0x%llx size %d at local "
+ "idx %d req_idx %d global_idx %d", __func__, acct->resp_desc->addr,
+ acct->resp_desc->len, acct->resp_idx, acct->req_idx, acct->idx);
+
+ if (write_mem(acct->resp_desc->addr, , acct->resp_desc->len)) {
+ log_warnx("%s: unable to write OK resp status data @ 0x%llx",
+ __func__, acct->resp_desc->addr);
+ goto free_rpl;
+ }
+
+ /* Move index for reply_rpl */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->resp_desc,
+ &(acct->resp_idx));
+
+ dprintf("%s: writing reply_rpl to 0x%llx size %d at "
+ "local idx %d req_idx %d global_idx %d",
+ __func__, acct->resp_desc->addr, acct->resp_desc->len,
+ acct->resp_idx, acct->req_idx, acct->idx);
+
+ if (write_mem(acct->resp_desc->addr, reply_rpl, acct->resp_desc->len)) {
+ log_warnx("%s: unable to write reply_rpl"
+ " response to gpa @ 0x%llx",
+ __func__, acct->resp_desc->addr);
+ } else {
+ ret = 1;
+ dev->cfg.isr_status = 1;
+ /* Move ring indexes */
+ vioscsi_next_ring_item(dev, acct->avail, acct->used,
+ acct->req_desc, acct->req_idx);
+ }
+
+free_rpl:
+ free(reply_rpl);
+rpl_out:
+ return (ret);
+}
+
+static int
vioscsi_handle_read_6(struct vioscsi_dev *dev,
struct virtio_scsi_req_hdr *req, struct virtio_vq_acct *acct)
{
@@ -2213,6 +2312,15 @@ vioscsi_notifyq(struct vioscsi_dev *dev)
break;
case MECHANISM_STATUS:
ret = vioscsi_handle_mechanism_status(dev, , );
+ if (ret) {
+ if (write_mem(q_gpa, vr, vr_sz)) {
+ log_warnx("%s: error writing vioring",
+ __func__);
+ }
+ }
+ break;
+ case REPORT_LUNS:
+ ret = vioscsi_handle_report_luns(dev, , );
if (ret) {
Re: VMD: vioscsi refactor
On Sun, Jan 14, 2018 at 06:08:05PM -0800, Mike Larkin wrote:
> On Sun, Jan 14, 2018 at 05:15:54PM -0800, Carlos Cardenas wrote:
> > Howdy.
> >
> > Attached is a patch that refactors the vioscsi driver (in particular,
> > the vioscsi_notifyq function).
> >
> > Each opcode is now handled in it's respective function
> > (vioscsi_handle_xxx). Which means, it's now easier to add more opcodes
> > and functionality to the driver. (Like supporting the new SeaBIOS
> > release)
> >
> > No functional changes have occurred and testing with various guests
> > confirms no regressions.
> >
> > Comments? Ok?
> >
> > +--+
> > Carlos
>
> This looks a lot better! Thanks carlos.
>
> Do you need me to walk through it or is this just moving deck chairs
> around? If the latter, go ahead.
>
> -ml
This is just musical chairs.
Will commit later this evening.
+--+
Carlos
>
>
> > Index: vioscsi.c
> > ===
> > RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
> > retrieving revision 1.1
> > diff -u -p -a -u -r1.1 vioscsi.c
> > --- vioscsi.c 3 Jan 2018 05:39:56 - 1.1
> > +++ vioscsi.c 15 Jan 2018 00:57:36 -
> > @@ -219,6 +219,1344 @@ vioscsi_finish_read(struct ioinfo *info)
> > return info->buf;
> > }
> >
> > +static int
> > +vioscsi_handle_tur(struct vioscsi_dev *dev, struct virtio_scsi_req_hdr
> > *req,
> > +struct virtio_vq_acct *acct)
> > +{
> > + int ret = 0;
> > + struct virtio_scsi_res_hdr resp;
> > +
> > + memset(, 0, sizeof(resp));
> > + /* Move index for response */
> > + acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->req_desc,
> > + &(acct->resp_idx));
> > +
> > + vioscsi_prepare_resp(, VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);
> > +
> > + if (write_mem(acct->resp_desc->addr, , acct->resp_desc->len)) {
> > + log_warnx("%s: unable to write OK resp status data @ 0x%llx",
> > + __func__, acct->resp_desc->addr);
> > + } else {
> > + ret = 1;
> > + dev->cfg.isr_status = 1;
> > + /* Move ring indexes */
> > + vioscsi_next_ring_item(dev, acct->avail, acct->used,
> > + acct->req_desc, acct->req_idx);
> > + }
> > +
> > + return (ret);
> > +}
> > +
> > +static int
> > +vioscsi_handle_inquiry(struct vioscsi_dev *dev,
> > +struct virtio_scsi_req_hdr *req, struct virtio_vq_acct *acct)
> > +{
> > + int ret = 0;
> > + struct virtio_scsi_res_hdr resp;
> > + uint16_t inq_len;
> > + struct scsi_inquiry *inq;
> > + struct scsi_inquiry_data *inq_data;
> > +
> > + memset(, 0, sizeof(resp));
> > + inq = (struct scsi_inquiry *)(req->cdb);
> > + inq_len = (uint16_t)_2btol(inq->length);
> > +
> > + log_debug("%s: INQ - EVPD %d PAGE_CODE 0x%08x LEN %d", __func__,
> > + inq->flags & SI_EVPD, inq->pagecode, inq_len);
> > +
> > + vioscsi_prepare_resp(,
> > + VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);
> > +
> > + inq_data = calloc(1, sizeof(struct scsi_inquiry_data));
> > +
> > + if (inq_data == NULL) {
> > + log_warnx("%s: cannot alloc inq_data", __func__);
> > + goto inq_out;
> > + }
> > +
> > + inq_data->device = T_CDROM;
> > + inq_data->dev_qual2 = SID_REMOVABLE;
> > + /* Leave version zero to say we don't comply */
> > + inq_data->response_format = INQUIRY_RESPONSE_FORMAT;
> > + inq_data->additional_length = SID_SCSI2_ALEN;
> > + memcpy(inq_data->vendor, INQUIRY_VENDOR, INQUIRY_VENDOR_LEN);
> > + memcpy(inq_data->product, INQUIRY_PRODUCT, INQUIRY_PRODUCT_LEN);
> > + memcpy(inq_data->revision, INQUIRY_REVISION, INQUIRY_REVISION_LEN);
> > +
> > + /* Move index for response */
> > + acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->req_desc,
> > + &(acct->resp_idx));
> > +
> > + dprintf("%s: writing resp to 0x%llx size %d at local "
> > + "idx %d req_idx %d global_idx %d", __func__, acct->resp_desc->addr,
> > + acct->resp_desc->len, acct->resp_idx, acct->req_idx, acct->idx);
> > +
> > + if (write_mem(acct->resp_desc->addr, , acct->resp_desc->len)) {
> > + log_warnx("%s: unable
VMD: vioscsi refactor
Howdy.
Attached is a patch that refactors the vioscsi driver (in particular,
the vioscsi_notifyq function).
Each opcode is now handled in it's respective function
(vioscsi_handle_xxx). Which means, it's now easier to add more opcodes
and functionality to the driver. (Like supporting the new SeaBIOS
release)
No functional changes have occurred and testing with various guests
confirms no regressions.
Comments? Ok?
+--+
Carlos
Index: vioscsi.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/vioscsi.c,v
retrieving revision 1.1
diff -u -p -a -u -r1.1 vioscsi.c
--- vioscsi.c 3 Jan 2018 05:39:56 - 1.1
+++ vioscsi.c 15 Jan 2018 00:57:36 -
@@ -219,6 +219,1344 @@ vioscsi_finish_read(struct ioinfo *info)
return info->buf;
}
+static int
+vioscsi_handle_tur(struct vioscsi_dev *dev, struct virtio_scsi_req_hdr *req,
+struct virtio_vq_acct *acct)
+{
+ int ret = 0;
+ struct virtio_scsi_res_hdr resp;
+
+ memset(, 0, sizeof(resp));
+ /* Move index for response */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->req_desc,
+ &(acct->resp_idx));
+
+ vioscsi_prepare_resp(, VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);
+
+ if (write_mem(acct->resp_desc->addr, , acct->resp_desc->len)) {
+ log_warnx("%s: unable to write OK resp status data @ 0x%llx",
+ __func__, acct->resp_desc->addr);
+ } else {
+ ret = 1;
+ dev->cfg.isr_status = 1;
+ /* Move ring indexes */
+ vioscsi_next_ring_item(dev, acct->avail, acct->used,
+ acct->req_desc, acct->req_idx);
+ }
+
+ return (ret);
+}
+
+static int
+vioscsi_handle_inquiry(struct vioscsi_dev *dev,
+struct virtio_scsi_req_hdr *req, struct virtio_vq_acct *acct)
+{
+ int ret = 0;
+ struct virtio_scsi_res_hdr resp;
+ uint16_t inq_len;
+ struct scsi_inquiry *inq;
+ struct scsi_inquiry_data *inq_data;
+
+ memset(, 0, sizeof(resp));
+ inq = (struct scsi_inquiry *)(req->cdb);
+ inq_len = (uint16_t)_2btol(inq->length);
+
+ log_debug("%s: INQ - EVPD %d PAGE_CODE 0x%08x LEN %d", __func__,
+ inq->flags & SI_EVPD, inq->pagecode, inq_len);
+
+ vioscsi_prepare_resp(,
+ VIRTIO_SCSI_S_OK, SCSI_OK, 0, 0, 0);
+
+ inq_data = calloc(1, sizeof(struct scsi_inquiry_data));
+
+ if (inq_data == NULL) {
+ log_warnx("%s: cannot alloc inq_data", __func__);
+ goto inq_out;
+ }
+
+ inq_data->device = T_CDROM;
+ inq_data->dev_qual2 = SID_REMOVABLE;
+ /* Leave version zero to say we don't comply */
+ inq_data->response_format = INQUIRY_RESPONSE_FORMAT;
+ inq_data->additional_length = SID_SCSI2_ALEN;
+ memcpy(inq_data->vendor, INQUIRY_VENDOR, INQUIRY_VENDOR_LEN);
+ memcpy(inq_data->product, INQUIRY_PRODUCT, INQUIRY_PRODUCT_LEN);
+ memcpy(inq_data->revision, INQUIRY_REVISION, INQUIRY_REVISION_LEN);
+
+ /* Move index for response */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->req_desc,
+ &(acct->resp_idx));
+
+ dprintf("%s: writing resp to 0x%llx size %d at local "
+ "idx %d req_idx %d global_idx %d", __func__, acct->resp_desc->addr,
+ acct->resp_desc->len, acct->resp_idx, acct->req_idx, acct->idx);
+
+ if (write_mem(acct->resp_desc->addr, , acct->resp_desc->len)) {
+ log_warnx("%s: unable to write OK resp status data @ 0x%llx",
+ __func__, acct->resp_desc->addr);
+ goto free_inq;
+ }
+
+ /* Move index for inquiry_data */
+ acct->resp_desc = vioscsi_next_ring_desc(acct->desc, acct->resp_desc,
+ &(acct->resp_idx));
+
+ dprintf("%s: writing inq_data to 0x%llx size %d at "
+ "local idx %d req_idx %d global_idx %d",
+ __func__, acct->resp_desc->addr, acct->resp_desc->len,
+ acct->resp_idx, acct->req_idx, acct->idx);
+
+ if (write_mem(acct->resp_desc->addr, inq_data, acct->resp_desc->len)) {
+ log_warnx("%s: unable to write inquiry"
+ " response to gpa @ 0x%llx",
+ __func__, acct->resp_desc->addr);
+ } else {
+ ret = 1;
+ dev->cfg.isr_status = 1;
+ /* Move ring indexes */
+ vioscsi_next_ring_item(dev, acct->avail, acct->used,
+ acct->req_desc, acct->req_idx);
+ }
+
+free_inq:
+ free(inq_data);
+inq_out:
+ return (ret);
+}
+
+static int
+vioscsi_handle_mode_sense(struct vioscsi_dev *dev,
+struct virtio_scsi_req_hdr *req, struct virtio_vq_acct *acct)
+{
+ int ret = 0;
+ struct virtio_scsi_res_hdr resp;
+ uint8_t mode_page_ctl;
+ uint8_t mode_page_code;
+ uint8_t *mode_reply;
+ uint8_t mode_reply_len;
+ struct
Re: vmd: allow vm with "cdrom" but no disk
On Wed, Jan 10, 2018 at 02:00:57PM +, Stuart Henderson wrote:
> Currently we require either "kernel" or "disk", but there may be
> some viable use cases where just a CDROM image is given. This adjusts
> the check to avoid bailing in that case.
>
> OK?
ok ccardenas
>
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 vmd.c
> --- vmd.c 8 Jan 2018 11:58:27 - 1.78
> +++ vmd.c 10 Jan 2018 13:57:26 -
> @@ -1140,8 +1140,9 @@ vm_register(struct privsep *ps, struct v
> } else if (vcp->vcp_nnics > VMM_MAX_NICS_PER_VM) {
> log_warnx("invalid number of interfaces");
> goto fail;
> - } else if (strlen(vcp->vcp_kernel) == 0 && vcp->vcp_ndisks == 0) {
> - log_warnx("no kernel or disk specified");
> + } else if (strlen(vcp->vcp_kernel) == 0 &&
> + vcp->vcp_ndisks == 0 && strlen(vcp->vcp_cdrom) == 0) {
> + log_warnx("no kernel or disk/cdrom specified");
> goto fail;
> } else if (strlen(vcp->vcp_name) == 0) {
> log_warnx("invalid VM name");
>
VMD: add regress tests for cdrom keyword
Attached are regress tests for the cdrom keyword.
Comments? Ok?
+--+
Carlos
Index: Makefile
===
RCS file: /home/los/cvs/src/regress/usr.sbin/vmd/config/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile11 Nov 2017 02:54:42 - 1.3
+++ Makefile7 Jan 2018 03:27:10 -
@@ -2,10 +2,10 @@
VMD ?= /usr/sbin/vmd
-VMD_PASS=boot-keyword memory-round memory-just-enough
+VMD_PASS=boot-keyword memory-round memory-just-enough cdrom-keyword
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
boot-name-too-long disk-path-too-long too-many-disks \
-switch-no-interface switch-no-add
+switch-no-interface switch-no-add cdrom-name-too-long
REGRESS_TARGETS=
Index: vmd-fail-cdrom-name-too-long.conf
===
RCS file: vmd-fail-cdrom-name-too-long.conf
diff -N vmd-fail-cdrom-name-too-long.conf
--- /dev/null 1 Jan 1970 00:00:00 -
+++ vmd-fail-cdrom-name-too-long.conf 7 Jan 2018 03:21:59 -
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail on cdrom path (> 128)
+iso="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/cd62.iso"
+vm "x" {
+cdrom $iso
+}
Index: vmd-fail-cdrom-name-too-long.ok
===
RCS file: vmd-fail-cdrom-name-too-long.ok
diff -N vmd-fail-cdrom-name-too-long.ok
--- /dev/null 1 Jan 1970 00:00:00 -
+++ vmd-fail-cdrom-name-too-long.ok 7 Jan 2018 03:21:59 -
@@ -0,0 +1 @@
+5: cdrom name too long
Index: vmd-pass-cdrom-keyword.conf
===
RCS file: vmd-pass-cdrom-keyword.conf
diff -N vmd-pass-cdrom-keyword.conf
--- /dev/null 1 Jan 1970 00:00:00 -
+++ vmd-pass-cdrom-keyword.conf 7 Jan 2018 03:21:59 -
@@ -0,0 +1,8 @@
+# $OpenBSD$
+# Pass on cdrom keyword
+
+vm "x" {
+memory 1G
+cdrom "cd62.iso"
+disable
+}
Index: vmd-pass-cdrom-keyword.ok
===
RCS file: vmd-pass-cdrom-keyword.ok
diff -N vmd-pass-cdrom-keyword.ok
--- /dev/null 1 Jan 1970 00:00:00 -
+++ vmd-pass-cdrom-keyword.ok 7 Jan 2018 03:21:59 -
@@ -0,0 +1 @@
+configuration OK
Re: VMD: revise check for regular files on disks
Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
> On Wed, Jan 03 2018, Carlos Cardenas <cardena...@gmail.com> wrote:
> > Howdy.
> >
> > Attached is a patch to address a TOCTOU issue with checking to
> > ensure disks are regular files, reported by jca@ .
> >
> > Comments? Ok?
>
> A bit late, but ok.
>
> While here, if the S_ISREG check fails there is no meaningful errno to
> report.
>
> ok?
>
ok ccardenas
>
> Index: config.c
> ===
> RCS file: /d/cvs/src/usr.sbin/vmd/config.c,v
> retrieving revision 1.39
> diff -u -p -p -u -r1.39 config.c
> --- config.c 4 Jan 2018 15:19:56 - 1.39
> +++ config.c 5 Jan 2018 07:24:41 -
> @@ -252,7 +252,7 @@ config_setvm(struct privsep *ps, struct
> goto fail;
> }
> if (S_ISREG(stat_buf.st_mode) == 0) {
> - log_warn("%s: cdrom %s is not a regular file", __func__,
> + log_warnx("%s: cdrom %s is not a regular file",
> __func__,
> vcp->vcp_cdrom);
> errno = VMD_CDROM_INVALID;
> goto fail;
> @@ -276,7 +276,7 @@ config_setvm(struct privsep *ps, struct
> goto fail;
> }
> if (S_ISREG(stat_buf.st_mode) == 0) {
> - log_warn("%s: disk %s is not a regular file", __func__,
> + log_warnx("%s: disk %s is not a regular file", __func__,
> vcp->vcp_disks[i]);
> errno = VMD_DISK_INVALID;
> goto fail;
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: VMD: revise check for regular files on disks
Mike Larkin <mlar...@azathoth.net> wrote:
> On Wed, Jan 03, 2018 at 08:03:56PM -0800, Carlos Cardenas wrote:
> > Howdy.
> >
> > Attached is a patch to address a TOCTOU issue with checking to
> > ensure disks are regular files, reported by jca@ .
> >
> > Comments? Ok?
> >
> > +--+
> > Carlos
>
> > Index: config.c
> > ===
> > RCS file: /home/los/cvs/src/usr.sbin/vmd/config.c,v
> > retrieving revision 1.38
> > diff -u -p -a -u -r1.38 config.c
> > --- config.c3 Jan 2018 05:39:56 - 1.38
> > +++ config.c4 Jan 2018 03:55:47 -
> > @@ -262,23 +262,23 @@ config_setvm(struct privsep *ps, struct
> > /* Open disk images for child */
> > for (i = 0 ; i < vcp->vcp_ndisks; i++) {
> > /* Stat disk[i] to ensure it is a regular file */
> > - if (stat(vcp->vcp_disks[i], _buf) == -1) {
> > + if ((diskfds[i] =
> > + open(vcp->vcp_disks[i], O_RDWR)) == -1) {
>
> O_RDONLY? Or do we actually support the SCSI write commands (ala
> writing ISO images?)
vcp_disks represent the vioblk devices which are RDWR.
vcp_cdrom is RDONLY since it doesn't support writing ISOs.
>
> > log_warn("%s: can't open disk %s", __func__,
> > vcp->vcp_disks[i]);
> > errno = VMD_DISK_MISSING;
> > goto fail;
> > }
> > - if (S_ISREG(stat_buf.st_mode) == 0) {
> > - log_warn("%s: disk %s is not a regular file", __func__,
> > + if (fstat(diskfds[i], _buf) == -1) {
> > + log_warn("%s: can't open disk %s", __func__,
> > vcp->vcp_disks[i]);
> > - errno = VMD_DISK_INVALID;
> > + errno = VMD_DISK_MISSING;
>
> I'd probably stick with INVALID here since technically the image is not
> really "missing"
Makes sense.
>
> > goto fail;
> > }
> > - if ((diskfds[i] =
> > - open(vcp->vcp_disks[i], O_RDWR)) == -1) {
> > - log_warn("%s: can't open disk %s", __func__,
> > + if (S_ISREG(stat_buf.st_mode) == 0) {
> > + log_warn("%s: disk %s is not a regular file", __func__,
> > vcp->vcp_disks[i]);
> > - errno = VMD_DISK_MISSING;
> > + errno = VMD_DISK_INVALID;
> > goto fail;
> > }
> > }
>
> ok mlarkin otherwise
VMD: revise check for regular files on disks
Howdy.
Attached is a patch to address a TOCTOU issue with checking to
ensure disks are regular files, reported by jca@ .
Comments? Ok?
+--+
Carlos
Index: config.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.38
diff -u -p -a -u -r1.38 config.c
--- config.c3 Jan 2018 05:39:56 - 1.38
+++ config.c4 Jan 2018 03:55:47 -
@@ -262,23 +262,23 @@ config_setvm(struct privsep *ps, struct
/* Open disk images for child */
for (i = 0 ; i < vcp->vcp_ndisks; i++) {
/* Stat disk[i] to ensure it is a regular file */
- if (stat(vcp->vcp_disks[i], _buf) == -1) {
+ if ((diskfds[i] =
+ open(vcp->vcp_disks[i], O_RDWR)) == -1) {
log_warn("%s: can't open disk %s", __func__,
vcp->vcp_disks[i]);
errno = VMD_DISK_MISSING;
goto fail;
}
- if (S_ISREG(stat_buf.st_mode) == 0) {
- log_warn("%s: disk %s is not a regular file", __func__,
+ if (fstat(diskfds[i], _buf) == -1) {
+ log_warn("%s: can't open disk %s", __func__,
vcp->vcp_disks[i]);
- errno = VMD_DISK_INVALID;
+ errno = VMD_DISK_MISSING;
goto fail;
}
- if ((diskfds[i] =
- open(vcp->vcp_disks[i], O_RDWR)) == -1) {
- log_warn("%s: can't open disk %s", __func__,
+ if (S_ISREG(stat_buf.st_mode) == 0) {
+ log_warn("%s: disk %s is not a regular file", __func__,
vcp->vcp_disks[i]);
- errno = VMD_DISK_MISSING;
+ errno = VMD_DISK_INVALID;
goto fail;
}
}
[PATCH 1/2] VMD: vioscsi support for cdrom
M_DISK, vm->vm_vmid, diskfds[i],
@@ -372,6 +404,8 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
if (kernfd != -1)
close(kernfd);
+ if (cdromfd != -1)
+ close(cdromfd);
if (diskfds != NULL) {
for (i = 0; i < vcp->vcp_ndisks; i++)
close(diskfds[i]);
@@ -477,3 +511,28 @@ config_getif(struct privsep *ps, struct imsg *imsg)
errno = EINVAL;
return (-1);
}
+
+int
+config_getcdrom(struct privsep *ps, struct imsg *imsg)
+{
+ struct vmd_vm *vm;
+
+ errno = 0;
+ if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) {
+ errno = ENOENT;
+ return (-1);
+ }
+
+ if (imsg->fd == -1) {
+ log_debug("invalid cdrom id");
+ goto fail;
+ }
+
+ vm->vm_cdrom = imsg->fd;
+ return (0);
+ fail:
+ if (imsg->fd != -1)
+ close(imsg->fd);
+ errno = EINVAL;
+ return (-1);
+}
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index a0e96545923..f3a17d90b15 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -114,8 +114,8 @@ typedef struct {
%token INCLUDE ERROR
-%token ADD BOOT DISABLE DISK DOWN ENABLE GROUP INTERFACE LLADDR LOCAL LOCKED
-%token MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SWITCH UP VM VMID
+%token ADD BOOT CDROM DISABLE DISK DOWN ENABLE GROUP INTERFACE LLADDR LOCAL
+%token LOCKED MEMORY NIFS OWNER PATH PREFIX RDOMAIN SIZE SWITCH UP VM VMID
%token NUMBER
%token STRING
%typelladdr
@@ -405,6 +405,23 @@ vm_opts: disable {
free($2);
vmc.vmc_flags |= VMOP_CREATE_KERNEL;
}
+ | CDROM string {
+ if (vcp->vcp_cdrom[0] != '\0') {
+ yyerror("cdrom specified more than once");
+ free($2);
+ YYERROR;
+
+ }
+ if (strlcpy(vcp->vcp_cdrom, $2,
+ sizeof(vcp->vcp_cdrom)) >=
+ sizeof(vcp->vcp_cdrom)) {
+ yyerror("cdrom name too long");
+ free($2);
+ YYERROR;
+ }
+ free($2);
+ vmc.vmc_flags |= VMOP_CREATE_CDROM;
+ }
| NIFS NUMBER {
if (vcp->vcp_nnics != 0) {
yyerror("interfaces specified more than once");
@@ -649,6 +666,7 @@ lookup(char *s)
static const struct keywords keywords[] = {
{ "add",ADD },
{ "boot", BOOT },
+ { "cdrom", CDROM },
{ "disable",DISABLE },
{ "disk", DISK },
{ "down", DOWN },
diff --git usr.sbin/vmd/vioscsi.c usr.sbin/vmd/vioscsi.c
new file mode 100644
index 000..f2e8af60b54
--- /dev/null
+++ usr.sbin/vmd/vioscsi.c
@@ -0,0 +1,1204 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2017 Carlos Cardenas <ccarde...@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "vmd.h"
+#include "vioscsi.h"
+#include "virtio.h"
+
+extern char *__progname;
+
+static void
+vioscsi_prepare_resp(struct virtio_scsi_res_hdr *resp, uint8_t vio_status,
+uint8_t scsi_status)
+{
+ /* Set status and response fields,8bit to OK*/
+ resp->response &= 0xFF00;
+ resp->response |= vio_status;
+ resp->status &= 0xFF00;
+ resp->status |= scsi_status;
+}
+
+static struct vring_desc*
+vioscsi_next_ring_desc(struct vring_desc* desc, struct vring_desc* cur,
+uint16_t *idx)
+{
+ *idx = cu
[PATCH 0/2] Add initial support for cdrom using vioscsi
This patchset adds the initial support for a vioscsi cdrom. Tests were done using: * large ( > 4GB) and small isos * OpenBSD as the guest (primary testing) * Seabios as the kernel (primary testing) It is expected that there's a possibility of adding more opcodes as the set of testing VMs increases. Without changes to Seabios (vmm-firmware) to enable vioscsi, it is only possible to mount the cdrom inside the guest. To test installing OpenBSD using the install sets on cdrom: vmctl start 'bsd' -b 'bsd.rd' -r 'install62.iso' -d 'disk.img' Comments? Ok? -- 2.15.0
[PATCH 2/2] VMD: add regress tests for cdrom keyword
Add regress tests for cdrom keyword
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
index 2adc69ae491..91e19037b9c 100644
--- regress/usr.sbin/vmd/config/Makefile
+++ regress/usr.sbin/vmd/config/Makefile
@@ -2,10 +2,10 @@
VMD ?= /usr/sbin/vmd
-VMD_PASS=boot-keyword memory-round memory-just-enough
+VMD_PASS=boot-keyword memory-round memory-just-enough cdrom-keyword
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
boot-name-too-long disk-path-too-long too-many-disks \
-switch-no-interface
+switch-no-interface cdrom-name-too-long
REGRESS_TARGETS=
diff --git regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.conf
regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.conf
new file mode 100644
index 000..6c47683cf3e
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail on cdrom path (> 128)
+iso="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/cd62.iso"
+vm "x" {
+cdrom $iso
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.ok
regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.ok
new file mode 100644
index 000..e94f1e3ef8c
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-cdrom-name-too-long.ok
@@ -0,0 +1 @@
+5: cdrom name too long
diff --git regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.conf
regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.conf
new file mode 100644
index 000..0f34b590c4a
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.conf
@@ -0,0 +1,8 @@
+# $OpenBSD$
+# Pass on cdrom keyword
+
+vm "x" {
+memory 1G
+cdrom "cd62.iso"
+disable
+}
diff --git regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.ok
regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.ok
new file mode 100644
index 000..403d828b763
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-pass-cdrom-keyword.ok
@@ -0,0 +1 @@
+configuration OK
--
2.15.0
[PATCH] update struct vm_create_params
Add cdrom entry to vm_create_params in preparation for cdrom
support in vmd.
Ok?
diff --git sys/arch/amd64/include/vmmvar.h sys/arch/amd64/include/vmmvar.h
index 4847fa3defa..0e067f3f49d 100644
--- sys/arch/amd64/include/vmmvar.h
+++ sys/arch/amd64/include/vmmvar.h
@@ -26,6 +26,7 @@
#define VMM_MAX_MEM_RANGES 16
#define VMM_MAX_DISKS_PER_VM 4
#define VMM_MAX_PATH_DISK 128
+#define VMM_MAX_PATH_CDROM 128
#define VMM_MAX_NAME_LEN 32
#define VMM_MAX_KERNEL_PATH128
#define VMM_MAX_VCPUS_PER_VM 64
@@ -419,6 +420,7 @@ struct vm_create_params {
size_t vcp_nnics;
struct vm_mem_range vcp_memranges[VMM_MAX_MEM_RANGES];
char
vcp_disks[VMM_MAX_DISKS_PER_VM][VMM_MAX_PATH_DISK];
+ charvcp_cdrom[VMM_MAX_PATH_CDROM];
charvcp_name[VMM_MAX_NAME_LEN];
charvcp_kernel[VMM_MAX_KERNEL_PATH];
uint8_t vcp_macs[VMM_MAX_NICS_PER_VM][6];
diff --git sys/arch/i386/include/vmmvar.h sys/arch/i386/include/vmmvar.h
index 32ba1234800..640120a7ab4 100644
--- sys/arch/i386/include/vmmvar.h
+++ sys/arch/i386/include/vmmvar.h
@@ -25,6 +25,7 @@
#define VMM_MAX_MEM_RANGES 16
#define VMM_MAX_DISKS_PER_VM 4
#define VMM_MAX_PATH_DISK 128
+#define VMM_MAX_PATH_CDROM 128
#define VMM_MAX_NAME_LEN 32
#define VMM_MAX_KERNEL_PATH128
#define VMM_MAX_VCPUS_PER_VM 64
@@ -357,6 +358,7 @@ struct vm_create_params {
size_t vcp_nnics;
struct vm_mem_range vcp_memranges[VMM_MAX_MEM_RANGES];
char
vcp_disks[VMM_MAX_DISKS_PER_VM][VMM_MAX_PATH_DISK];
+ charvcp_cdrom[VMM_MAX_PATH_CDROM];
charvcp_name[VMM_MAX_NAME_LEN];
charvcp_kernel[VMM_MAX_KERNEL_PATH];
uint8_t vcp_macs[VMM_MAX_NICS_PER_VM][6];
--
2.15.0
[PATCH] scsi style(9) fix for SC_DEBUG{N} macros
Patch fixes a broken compilation when using SCSIDEBUG options(4).
Resolved with krw@ assistance.
Ok?
diff --git sys/scsi/scsi_debug.h sys/scsi/scsi_debug.h
index bf1519fa36d..56c2d530653 100644
--- sys/scsi/scsi_debug.h
+++ sys/scsi/scsi_debug.h
@@ -40,15 +40,17 @@ extern int scsidebug_level;
* This is the usual debug macro for use with the above bits
*/
#ifdef SCSIDEBUG
-#defineSC_DEBUG(link,Level,Printstuff) \
+#defineSC_DEBUG(link,Level,Printstuff) do {\
if ((link)->flags & (Level)) { \
sc_print_addr(link);\
printf Printstuff; \
- }
-#defineSC_DEBUGN(link,Level,Printstuff) \
+ } \
+} while (0)
+#defineSC_DEBUGN(link,Level,Printstuff) do {\
if ((link)->flags & (Level)) { \
printf Printstuff; \
- }
+ } \
+} while (0)
#else
#define SC_DEBUG(A,B,C)
#define SC_DEBUGN(A,B,C)
--
2.14.3
[PATCH 2/2 v3] VMD: regress tests update for switch configuration
Update regression tests to match new switch configuration
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
index 2adc69ae491..3bf124aff56 100644
--- regress/usr.sbin/vmd/config/Makefile
+++ regress/usr.sbin/vmd/config/Makefile
@@ -5,7 +5,7 @@ VMD ?= /usr/sbin/vmd
VMD_PASS=boot-keyword memory-round memory-just-enough
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
boot-name-too-long disk-path-too-long too-many-disks \
-switch-no-interface
+switch-no-interface switch-no-add
REGRESS_TARGETS=
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
new file mode 100644
index 000..40117749346
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail when a switch is attempting to use add
+switch "x" {
+interface bridge0
+add vether0
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
new file mode 100644
index 000..c04c2d13732
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
@@ -0,0 +1 @@
+5: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
index 891d9c88176..f92c09656d6 100644
--- regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
@@ -1,5 +1,5 @@
# $OpenBSD: vmd-fail-switch-no-interface.conf,v 1.1 2017/10/30 03:49:30
mlarkin Exp $
# Fail when a switch is missing interface name
switch "x" {
-add vether0
+up
}
--
2.14.3
[PATCH 0/2 v3] VMD: switch configuration changes
This patch series makes the following changes to switch configuration: * Removes adding static interfaces (done in /etc/hostname.if) * vm->interface->rdomain take precedence over sw->rdomain Updated regression tests to match vm.conf changes. Updated examples/vm.conf to match vm.conf changes. changes since v2: * style changes from reyk@ -- 2.14.3
[PATCH 1/2 v3] VMD: remove add from switch configuration
Remove configuration items on switches for:
* adding static interfaces
Adding static interfaces are to be set in hostname.if.
Changed rule on rdomain:
* vm->interface->rdomain takes precedence over sw->rdomain
Update examples/vm.conf and vm.conf manpage to reflect changes.
Comments? Ok?
Ok by reyk@
diff --git etc/examples/vm.conf etc/examples/vm.conf
index f35235e3fba..d61ce8c4977 100644
--- etc/examples/vm.conf
+++ etc/examples/vm.conf
@@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
#
switch "uplink" {
- # This interface will default to bridge0, but switch(4) is supported
+ # This switch will use bridge0, defined by /etc/hostname.bridge0, as
+ # the underlying interface. switch(4) is also supported
#interface switch0
-
- # Add additional members
- add em0
+ interface bridge0
}
switch "local" {
- add vether0
+ interface bridge1
down
}
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index a0e96545923..18543a74911 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -88,7 +88,6 @@ intparse_disk(char *);
static struct vmop_create_params vmc;
static struct vm_create_params *vcp;
static struct vmd_switch *vsw;
-static struct vmd_if *vif;
static struct vmd_vm *vm;
static char vsw_type[IF_NAMESIZE];
static int vcp_disable;
@@ -193,7 +192,6 @@ switch : SWITCH string {
vsw->sw_id = env->vmd_nswitches + 1;
vsw->sw_name = $2;
vsw->sw_flags = VMIFF_UP;
- TAILQ_INIT(>sw_ifs);
vcp_disable = 0;
} '{' optnl switch_opts_l '}' {
@@ -224,21 +222,6 @@ switch_opts_l : switch_opts_l switch_opts nl
switch_opts: disable {
vcp_disable = $1;
}
- | ADD string{
- chartype[IF_NAMESIZE];
-
- if ((vif = calloc(1, sizeof(*vif))) == NULL)
- fatal("could not allocate interface");
-
- if (priv_getiftype($2, type, NULL) == -1) {
- yyerror("invalid interface: %s", $2);
- free($2);
- YYERROR;
- }
- vif->vif_name = $2;
-
- TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry);
- }
| GROUP string {
if (priv_validgroup($2) == -1) {
yyerror("invalid group name: %s", $2);
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index d585bf75a99..c1d0c0e13be 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -255,9 +255,18 @@ priv_validgroup(const char *name)
}
/*
- * Called from the process peer
+ * Called from the Parent process to setup vm interface(s)
+ * - ensure the interface has the description set (tracking purposes)
+ * - if interface is to be attached to a switch, attach it
+ * - check if rdomain is set on interface and switch
+ * - if interface only or both, use interface rdomain
+ * - if switch only, use switch rdomain
+ * - check if group is set on interface and switch
+ * - if interface, add it
+ * - if switch, add it
+ * - ensure the interface is up/down
+ * - if local interface, set address
*/
-
int
vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
{
@@ -279,18 +288,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
return (-1);
- /* Use the configured rdomain or get it from the process */
- if (vif->vif_flags & VMIFF_RDOMAIN)
- vfr.vfr_id = vif->vif_rdomain;
- else
- vfr.vfr_id = getrtable();
- if (vfr.vfr_id != 0)
- log_debug("%s: interface %s rdomain %u", __func__,
- vfr.vfr_name, vfr.vfr_id);
-
- proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
- , sizeof(vfr));
-
/* Description can be truncated */
(void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value),
"vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name);
@@ -301,8 +298,17 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
, sizeof(vfr));
- /* Add interface to bridge/switch */
- if ((vsw = switch_getbyname(vif->vif_switch)) != NULL) {
+ /* set default rdomain */
+ vfr.vfr_id = getrtable();
+
+ vsw = switch_getbyname(vif->vif_switch);
+
+
Re: vmd: add minimal DHCP support
On 11/02/17 14:26, Reyk Floeter wrote:
Hi,
the problem got reported a few times and a similar diff was floating
around: vmd's "local interface" implements a simple auto-magic BOOTP
server, but udhcpc doesn't support BOOTP, which is the predecessor and
a valid subset of DHCP. udhcpc is used by busybox and many embedded
Linux distributions, maybe even by Android.
The following diff adds minimal DHCP support which works with udhcpc.
OK?
Reyk
Nice patch. +1
+--+
Carlos
Index: usr.sbin/vmd/dhcp.c
===
RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 dhcp.c
--- usr.sbin/vmd/dhcp.c 24 Apr 2017 07:14:27 - 1.3
+++ usr.sbin/vmd/dhcp.c 2 Nov 2017 21:15:03 -
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -38,12 +39,13 @@ extern struct vmd *env;
ssize_t
dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf)
{
- unsigned char *respbuf = NULL;
+ unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0;
ssize_t offset, respbuflen = 0;
struct packet_ctxpc;
struct dhcp_packet req, resp;
- struct in_addr in, mask;
+ struct in_addr server_addr, mask, client_addr, requested_addr;
size_t resplen, o;
+ uint32_t ltime;
if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header)))
return (-1);
@@ -76,24 +78,54 @@ dhcp_request(struct vionet_dev *dev, cha
if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0)
return (-1);
+ /* Get a few DHCP options (best effort as we fall back to BOOTP) */
+ if (memcmp(,
+ DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) {
+ memset(_addr, 0, sizeof(requested_addr));
+ op = req.options + DHCP_OPTIONS_COOKIE_LEN;
+ oe = req.options + sizeof(req.options);
+ while (*op != DHO_END && op < oe) {
+ if (op[0] == DHO_PAD) {
+ op++;
+ continue;
+ }
+ if (op + 1 + op[1] >= oe)
+ break;
+ if (op[0] == DHO_DHCP_MESSAGE_TYPE &&
+ op[1] == 1)
+ dhcptype = op[2];
+ else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS &&
+ op[1] == sizeof(requested_addr))
+ memcpy(_addr, [2],
+ sizeof(requested_addr));
+ op += 2 + op[1];
+ }
+ }
+
memset(, 0, sizeof(resp));
resp.op = BOOTREPLY;
resp.htype = req.htype;
resp.hlen = req.hlen;
resp.xid = req.xid;
- if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix,
+ if ((client_addr.s_addr =
+ vm_priv_addr(>vmd_cfg.cfg_localprefix,
dev->vm_vmid, dev->idx, 1)) == 0)
return (-1);
- memcpy(, , sizeof(in));
- memcpy((_dst)->sin_addr, , sizeof(in));
+ memcpy(, _addr,
+ sizeof(client_addr));
+ memcpy((_dst)->sin_addr, _addr,
+ sizeof(client_addr));
ss2sin(_dst)->sin_port = htons(CLIENT_PORT);
- if ((in.s_addr = vm_priv_addr(>vmd_cfg.cfg_localprefix,
+ if ((server_addr.s_addr =
+ vm_priv_addr(>vmd_cfg.cfg_localprefix,
dev->vm_vmid, dev->idx, 0)) == 0)
return (-1);
- memcpy(, , sizeof(in));
- memcpy((_src)->sin_addr, , sizeof(in));
+ memcpy(, _addr,
+ sizeof(server_addr));
+ memcpy((_src)->sin_addr, _addr,
+ sizeof(server_addr));
ss2sin(_src)->sin_port = htons(SERVER_PORT);
/* Packet is already allocated */
@@ -124,6 +156,44 @@ dhcp_request(struct vionet_dev *dev, cha
DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN);
o+= DHCP_OPTIONS_COOKIE_LEN;
+ /* Did we receive a DHCP request or was it just BOOTP? */
+ if (dhcptype) {
+ /*
+* There is no need for a real state machine as we always
+* answer with the same client IP and options for the VM.
+*/
+ if (dhcptype == DHCPDISCOVER)
+ dhcptype = DHCPOFFER;
+ else if (dhcptype == DHCPREQUEST &&
+ (requested_addr.s_addr == 0 ||
+ client_addr.s_addr == requested_addr.s_addr))
+ dhcptype = DHCPACK;
+ else
+ dhcptype = DHCPNAK;
+
+ resp.options[o++] = DHO_DHCP_MESSAGE_TYPE;
+ resp.options[o++] = sizeof(dhcptype);
+ memcpy([o], , sizeof(dhcptype));
+
[PATCH] VMD: remove debug message in proc
This debug message has served its duty for finding bugs in
shutdown related issues.
Ok reyk@
diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c
index afa9b223649..d223d6bcce2 100644
--- usr.sbin/vmd/proc.c
+++ usr.sbin/vmd/proc.c
@@ -756,8 +756,6 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid
id, int n,
proc_range(ps, id, , );
for (; n < m; n++) {
- log_debug("%s: about to compose_event to proc %d",
- __func__, id);
if (imsg_compose_event(>ps_ievs[id][n],
type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
return (-1);
--
2.14.3
[PATCH 1/2 v2] VMD: remove add from switch configuration
Remove configuration items on switches for:
* adding static interfaces
Adding static interfaces are to be set in hostname.if.
Changed rule on rdomain:
* vm->interface->rdomain takes precedence over sw->rdomain
Update examples/vm.conf and vm.conf manpage to reflect changes.
Comments? Ok?
diff --git etc/examples/vm.conf etc/examples/vm.conf
index f35235e3fba..d61ce8c4977 100644
--- etc/examples/vm.conf
+++ etc/examples/vm.conf
@@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
#
switch "uplink" {
- # This interface will default to bridge0, but switch(4) is supported
+ # This switch will use bridge0, defined by /etc/hostname.bridge0, as
+ # the underlying interface. switch(4) is also supported
#interface switch0
-
- # Add additional members
- add em0
+ interface bridge0
}
switch "local" {
- add vether0
+ interface bridge1
down
}
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index a0e96545923..18543a74911 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -88,7 +88,6 @@ intparse_disk(char *);
static struct vmop_create_params vmc;
static struct vm_create_params *vcp;
static struct vmd_switch *vsw;
-static struct vmd_if *vif;
static struct vmd_vm *vm;
static char vsw_type[IF_NAMESIZE];
static int vcp_disable;
@@ -193,7 +192,6 @@ switch : SWITCH string {
vsw->sw_id = env->vmd_nswitches + 1;
vsw->sw_name = $2;
vsw->sw_flags = VMIFF_UP;
- TAILQ_INIT(>sw_ifs);
vcp_disable = 0;
} '{' optnl switch_opts_l '}' {
@@ -224,21 +222,6 @@ switch_opts_l : switch_opts_l switch_opts nl
switch_opts: disable {
vcp_disable = $1;
}
- | ADD string{
- chartype[IF_NAMESIZE];
-
- if ((vif = calloc(1, sizeof(*vif))) == NULL)
- fatal("could not allocate interface");
-
- if (priv_getiftype($2, type, NULL) == -1) {
- yyerror("invalid interface: %s", $2);
- free($2);
- YYERROR;
- }
- vif->vif_name = $2;
-
- TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry);
- }
| GROUP string {
if (priv_validgroup($2) == -1) {
yyerror("invalid group name: %s", $2);
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index d585bf75a99..c099bdf4ba5 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -255,7 +255,17 @@ priv_validgroup(const char *name)
}
/*
- * Called from the process peer
+ * Called from the Parent process to setup vm interface(s)
+ * - ensure the interface has the description set (tracking purposes)
+ * - if interface is to be attached to a switch, attach it
+ * - check if rdomain is set on interface and switch
+ * - if interface only or both, use interface rdomain
+ * - if switch only, use switch rdomain
+ * - check if group is set on interface and switch
+ * - if interface, add it
+ * - if switch, add it
+ * - ensure the interface is up/down
+ * - if local interface, set address
*/
int
@@ -279,18 +289,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
return (-1);
- /* Use the configured rdomain or get it from the process */
- if (vif->vif_flags & VMIFF_RDOMAIN)
- vfr.vfr_id = vif->vif_rdomain;
- else
- vfr.vfr_id = getrtable();
- if (vfr.vfr_id != 0)
- log_debug("%s: interface %s rdomain %u", __func__,
- vfr.vfr_name, vfr.vfr_id);
-
- proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
- , sizeof(vfr));
-
/* Description can be truncated */
(void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value),
"vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name);
@@ -301,8 +299,16 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
, sizeof(vfr));
- /* Add interface to bridge/switch */
- if ((vsw = switch_getbyname(vif->vif_switch)) != NULL) {
+ vsw = switch_getbyname(vif->vif_switch);
+ /* set default rdomain */
+ vfr.vfr_id = getrtable();
+
+ /* Check if switch should exist */
+ if (vsw == NULL &&
[PATCH 2/2 v2] VMD: regress tests update for switch configuration
Update regression tests to match new switch configuration
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
index 2adc69ae491..3bf124aff56 100644
--- regress/usr.sbin/vmd/config/Makefile
+++ regress/usr.sbin/vmd/config/Makefile
@@ -5,7 +5,7 @@ VMD ?= /usr/sbin/vmd
VMD_PASS=boot-keyword memory-round memory-just-enough
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
boot-name-too-long disk-path-too-long too-many-disks \
-switch-no-interface
+switch-no-interface switch-no-add
REGRESS_TARGETS=
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
new file mode 100644
index 000..40117749346
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail when a switch is attempting to use add
+switch "x" {
+interface bridge0
+add vether0
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
new file mode 100644
index 000..c04c2d13732
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
@@ -0,0 +1 @@
+5: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
index 891d9c88176..f92c09656d6 100644
--- regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
@@ -1,5 +1,5 @@
# $OpenBSD: vmd-fail-switch-no-interface.conf,v 1.1 2017/10/30 03:49:30
mlarkin Exp $
# Fail when a switch is missing interface name
switch "x" {
-add vether0
+up
}
--
2.14.3
[PATCH 0/2 v2] VMD: switch configuration changes
This patch series makes the following changes to switch configuration: * Removes adding static interfaces (done in /etc/hostname.if) * vm->interface->rdomain take precedence over sw->rdomain Updated regression tests to match vm.conf changes. Updated examples/vm.conf to match vm.conf changes. changes since v1: * v1 either broke some use cases or made some use cases more complicated to achieve (a.k.a "qemu-esque"); thanks to reyk@ for going through them with me. -- 2.14.3
[PATCH 0/2] VMD: complete switch configuration overhaul
This patch series finishes up the switch configuration to be handled in hostname.if change. It removes from switch configuration in vm.conf: * adding interfaces (done in /etc/hostname.if) * setting group(s) (done in /etc/hostname.if) * setting rdomain (don in /etc/hostname.if) vmd now queries the underlying interface for the switch user-defined group(s) and rdomain for propagation to vm's vifs that attach to it. Same functionality, different way to retrieve it. Updated regression tests to match vm.conf changes. Updated examples/vm.conf to match vm.conf changes. -- 2.14.3
[PATCH 1/2] VMD: complete switch configuration overhaul
Removed configuration items on switches for:
* adding interfaces
* setting group
* setting rdomain
All of these items are now to be set in hostname.if and are
queried by vmd for propagation to the vm's vif(s).
Update example vm.conf and vm.conf manpage to reflect changes.
Comments? Ok?
diff --git etc/examples/vm.conf etc/examples/vm.conf
index f35235e3fba..836a9f5a7c7 100644
--- etc/examples/vm.conf
+++ etc/examples/vm.conf
@@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
#
switch "uplink" {
- # This interface will default to bridge0, but switch(4) is supported
- #interface switch0
-
- # Add additional members
- add em0
+ # This switch will use bridge0, defined by /etc/hostname.bridge0, as
+ # the underlying interface. switch(4) is also supported as
+ # interface switchX
+ interface bridge0
}
switch "local" {
- add vether0
+ interface bridge1
down
}
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index a0e96545923..0bb1b0ceefa 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -88,7 +88,6 @@ intparse_disk(char *);
static struct vmop_create_params vmc;
static struct vm_create_params *vcp;
static struct vmd_switch *vsw;
-static struct vmd_if *vif;
static struct vmd_vm *vm;
static char vsw_type[IF_NAMESIZE];
static int vcp_disable;
@@ -193,7 +192,7 @@ switch : SWITCH string {
vsw->sw_id = env->vmd_nswitches + 1;
vsw->sw_name = $2;
vsw->sw_flags = VMIFF_UP;
- TAILQ_INIT(>sw_ifs);
+ TAILQ_INIT(>sw_group);
vcp_disable = 0;
} '{' optnl switch_opts_l '}' {
@@ -224,29 +223,6 @@ switch_opts_l : switch_opts_l switch_opts nl
switch_opts: disable {
vcp_disable = $1;
}
- | ADD string{
- chartype[IF_NAMESIZE];
-
- if ((vif = calloc(1, sizeof(*vif))) == NULL)
- fatal("could not allocate interface");
-
- if (priv_getiftype($2, type, NULL) == -1) {
- yyerror("invalid interface: %s", $2);
- free($2);
- YYERROR;
- }
- vif->vif_name = $2;
-
- TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry);
- }
- | GROUP string {
- if (priv_validgroup($2) == -1) {
- yyerror("invalid group name: %s", $2);
- free($2);
- YYERROR;
- }
- vsw->sw_group = $2;
- }
| INTERFACE string {
if (priv_getiftype($2, vsw_type, NULL) == -1 ||
priv_findname(vsw_type, vmd_descsw) == -1) {
@@ -266,14 +242,6 @@ switch_opts: disable {
| LOCKED LLADDR {
vsw->sw_flags |= VMIFF_LOCKED;
}
- | RDOMAIN NUMBER{
- if ($2 < 0 || $2 > RT_TABLEID_MAX) {
- yyerror("invalid rdomain: %lld", $2);
- YYERROR;
- }
- vsw->sw_flags |= VMIFF_RDOMAIN;
- vsw->sw_rdomain = $2;
- }
| updown{
if ($1)
vsw->sw_flags |= VMIFF_UP;
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index d585bf75a99..6253ccdd785 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -44,6 +44,9 @@
#include "proc.h"
#include "vmd.h"
+/* if groups to ignore */
+const char *ifgroup_ignore[] = { "all", "bridge", "switch", NULL };
+
int priv_dispatch_parent(int, struct privsep_proc *, struct imsg *);
voidpriv_run(struct privsep *, struct privsep_proc *, void *);
@@ -264,6 +267,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
struct vmd *env = ps->ps_env;
struct vm_create_params *vcp = >vm_params.vmc_params;
struct vmd_if *vif;
+ struct vmd_ifgr *vifgrp;
struct vmd_switch *vsw;
unsigned int i;
struct vmop_ifreqvfr, vfbr;
@@ -279,18 +283,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
return (-1);
- /* Use the configured rdomain or get it from
[PATCH 2/2] VMD: regress tests update for switch configuration
Update regression tests to match new switch configuration
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
index 2adc69ae491..a337b816acc 100644
--- regress/usr.sbin/vmd/config/Makefile
+++ regress/usr.sbin/vmd/config/Makefile
@@ -5,7 +5,8 @@ VMD ?= /usr/sbin/vmd
VMD_PASS=boot-keyword memory-round memory-just-enough
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
boot-name-too-long disk-path-too-long too-many-disks \
-switch-no-interface
+switch-no-interface switch-no-add switch-no-group \
+switch-no-rdomain
REGRESS_TARGETS=
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
new file mode 100644
index 000..58b6ea699f7
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.conf
@@ -0,0 +1,5 @@
+# $OpenBSD$
+# Fail when a switch is attempting to add an interface
+switch "x" {
+add vether0
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
new file mode 100644
index 000..219cba51f0b
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-add.ok
@@ -0,0 +1 @@
+4: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf
new file mode 100644
index 000..a3fbc27b086
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail when a switch is attempting to set a group
+switch "x" {
+interface bridge0
+group test
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok
new file mode 100644
index 000..c04c2d13732
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-group.ok
@@ -0,0 +1 @@
+5: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
index 891d9c88176..f92c09656d6 100644
--- regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
@@ -1,5 +1,5 @@
# $OpenBSD: vmd-fail-switch-no-interface.conf,v 1.1 2017/10/30 03:49:30
mlarkin Exp $
# Fail when a switch is missing interface name
switch "x" {
-add vether0
+up
}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf
new file mode 100644
index 000..ade16261abb
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.conf
@@ -0,0 +1,5 @@
+# $OpenBSD$
+# Fail when a switch is attempting to specify a rdomain
+switch "x" {
+rdomain 1
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok
new file mode 100644
index 000..219cba51f0b
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-rdomain.ok
@@ -0,0 +1 @@
+4: syntax error
--
2.14.3
[PATCH 2/2] VMD: update regress tests
* Update regress tests for new requirement (interface name on switches)
* Add new test for interface name on switch
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
index 16a43066415..68b5c13323a 100644
--- regress/usr.sbin/vmd/config/Makefile
+++ regress/usr.sbin/vmd/config/Makefile
@@ -4,7 +4,8 @@ VMD ?= /usr/sbin/vmd
VMD_PASS=boot-keyword memory-round memory-just-enough
VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
-boot-name-too-long disk-path-too-long too-many-disks
+boot-name-too-long disk-path-too-long too-many-disks \
+switch-no-interface
REGRESS_TARGETS=
diff --git regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
index 427e5b2a015..77b1a341e44 100644
--- regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
+++ regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
@@ -2,6 +2,7 @@
# Fail on kernel keyword; has been replaced by boot.
ramdisk="/bsd.rd"
switch "sw" {
+interface bridge0
add vether0
}
vm "x" {
diff --git regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
index 348817b1477..c171570d330 100644
--- regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
+++ regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
@@ -1 +1 @@
-8: syntax error
+9: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
new file mode 100644
index 000..8afa9e91729
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.conf
@@ -0,0 +1,5 @@
+# $OpenBSD$
+# Fail when a switch is missing interface name
+switch "x" {
+add vether0
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.ok
regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.ok
new file mode 100644
index 000..1cdd5c14c03
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-switch-no-interface.ok
@@ -0,0 +1 @@
+5: switch "x" is missing interface name
diff --git regress/usr.sbin/vmd/config/vmd-pass-boot-keyword.conf
regress/usr.sbin/vmd/config/vmd-pass-boot-keyword.conf
index 84124319e1e..2443c697ad0 100644
--- regress/usr.sbin/vmd/config/vmd-pass-boot-keyword.conf
+++ regress/usr.sbin/vmd/config/vmd-pass-boot-keyword.conf
@@ -2,6 +2,7 @@
# Pass on boot keyword as it has replaced the kernel keyword.
ramdisk="/bsd.rd"
switch "sw" {
+interface bridge0
add vether0
}
vm "x" {
--
2.14.2
[PATCH 1/2] VMD: Require interface to be defined in switches
* Require interface name to be defined for switches in vm.conf
** Requires user to create bridge(4) beforehand
* Remove code to create bridges on the fly
* Add code to ensure bridge really exists
* Update manpage switch and example sections
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index 55a9b0c7acc..a0b21fa8ef1 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -90,7 +90,6 @@ static struct vm_create_params*vcp;
static struct vmd_switch *vsw;
static struct vmd_if *vif;
static struct vmd_vm *vm;
-static unsigned int vsw_unit;
static char vsw_type[IF_NAMESIZE];
static int vcp_disable;
static size_t vcp_nnics;
@@ -194,12 +193,17 @@ switch: SWITCH string {
vsw->sw_id = env->vmd_nswitches + 1;
vsw->sw_name = $2;
vsw->sw_flags = VMIFF_UP;
- snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname),
- "%s%u", vsw_type, vsw_unit++);
TAILQ_INIT(>sw_ifs);
vcp_disable = 0;
} '{' optnl switch_opts_l '}' {
+ if (strnlen(vsw->sw_ifname,
+ sizeof(vsw->sw_ifname)) == 0) {
+ yyerror("switch \"%s\" is missing interface
name",
+ vsw->sw_name);
+ YYERROR;
+ }
+
if (vcp_disable) {
log_debug("%s:%d: switch \"%s\""
" skipped (disabled)",
@@ -244,13 +248,12 @@ switch_opts : disable {
vsw->sw_group = $2;
}
| INTERFACE string {
- if (priv_getiftype($2, vsw_type, _unit) == -1 ||
+ if (priv_getiftype($2, vsw_type, NULL) == -1 ||
priv_findname(vsw_type, vmd_descsw) == -1) {
yyerror("invalid switch interface: %s", $2);
free($2);
YYERROR;
}
- vsw_unit++;
if (strlcpy(vsw->sw_ifname, $2,
sizeof(vsw->sw_ifname)) >= sizeof(vsw->sw_ifname)) {
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index ef42549d105..d66bdcc9b4f 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -87,8 +87,8 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct
imsg *imsg)
switch (imsg->hdr.type) {
case IMSG_VMDOP_PRIV_IFDESCR:
- case IMSG_VMDOP_PRIV_IFCREATE:
case IMSG_VMDOP_PRIV_IFRDOMAIN:
+ case IMSG_VMDOP_PRIV_IFEXISTS:
case IMSG_VMDOP_PRIV_IFADD:
case IMSG_VMDOP_PRIV_IFUP:
case IMSG_VMDOP_PRIV_IFDOWN:
@@ -118,13 +118,6 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
struct imsg *imsg)
if (ioctl(env->vmd_fd, SIOCSIFDESCR, ) < 0)
log_warn("SIOCSIFDESCR");
break;
- case IMSG_VMDOP_PRIV_IFCREATE:
- /* Create the bridge if it doesn't exist */
- strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
- if (ioctl(env->vmd_fd, SIOCIFCREATE, ) < 0 &&
- errno != EEXIST)
- log_warn("SIOCIFCREATE");
- break;
case IMSG_VMDOP_PRIV_IFRDOMAIN:
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
ifr.ifr_rdomainid = vfr.vfr_id;
@@ -145,6 +138,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
struct imsg *imsg)
errno != EEXIST)
log_warn("SIOCBRDGADD");
break;
+ case IMSG_VMDOP_PRIV_IFEXISTS:
+ /* Determine if bridge/switch exists */
+ strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
+ if (ioctl(env->vmd_fd, SIOCGIFFLAGS, ) < 0)
+ fatalx("%s: bridge \"%s\" does not exist",
+ __func__, vfr.vfr_name);
+ break;
case IMSG_VMDOP_PRIV_IFUP:
case IMSG_VMDOP_PRIV_IFDOWN:
/* Set the interface status */
@@ -319,10 +319,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
log_debug("%s: interface %s add %s", __func__,
vfbr.vfr_name, vfbr.vfr_value);
- proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE,
- , sizeof(vfbr));
- proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
- , sizeof(vfbr));
proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFADD,
[PATCH 0/2] VMD: require interface to be defined in switches
This patch set changes the behavior of switches in vm.conf by requiring an interface name to be defined (previously, it was optional). This change also removes the responsibility of creating the underlying bridge from vmd to the user (i.e. doas ifconfig bridge0 create). These changes allow vmctl reload/reset to happen without error of adding/removing bridge ports and cluttering up the system with empty bridges. Included with the patch set are updated regression tests for vmd. Comments? Ok? +--+ Carlos -- 2.14.2
Re: [PATCH] VMD: Remove switch on reload prior to re-creating
On 10/10/17 23:31, Claudio Jeker wrote:
> On Tue, Oct 10, 2017 at 10:52:42PM -0700, Mike Larkin wrote:
>> On Wed, Oct 11, 2017 at 06:46:22AM +0200, Claudio Jeker wrote:
>>> On Tue, Oct 10, 2017 at 03:57:25PM -0700, Carlos Cardenas wrote:
>>>> Destroy switch on `vmctl reload` to allow SIOCBRDGADD to succeed
>>>> when creating new bridge and attaching interfaces to it.
>>>>
>>>> Comments? Ok?
>>>
>>> I don't think it is a good idea to destroy and recreate bridge interfaces
>>> on every reload. This changes the underlying network and may result in
>>> broken connections and network hickups. I would suggest that vmctl is
>>> instead checking which interfaces are on the bridge and calls SIOCBRDDEL
>>> if needed and skips the SIOCBRDGADD if not needed.
>>>
>>
>> you mean vmd, right?
>>
>> vmctl has no special privilege to do this.
>
> yeah, sorry, was not fully awake yet. vmd needs to be smarter on config
> reload IMO. Destroying interfaces for no reason is a bad practice.
>
We can definitely make things smarter. For my sake, I'm going to go through a
couple current scenarios to make sure I'm picking up what you're putting down.
1) vm.conf has the following switch definition
switch "a" {
add vether0
}
On vmd startup, a bridge(4) is created (bridge0 at first) and vether0 is added
to it. On reset/reload, any running vms are stopped and the switch is removed
prior to re-reading the config file and creating new bridges/vms. This means,
the new bridge is now bridge1 and vether0 will be added.
Today, this scenario fails since vether0 was never removed from bridge0 to be
added to bridge1. (what the patch addresses).
2) vm.conf has the following definition
switch "a" {
interface "bridge0"
add vether0
}
Just like 1) except this time after any reset/reload, the switch name is
always bridge0. This seems to be way a switch(4) is used in vmd.
In scenario 1, are you proposing we keep bridge0 (and other predecessors around)
and only remove/add the bridge ports (vether0 in this case) to the current
bridge{n}? If so, should we use the bridge ports defined in vm.conf or from
SIOCBRDGIFS since they could be different?
How should we handle scenario 2 since my patch is kinda heavy handed for it?
Another option would be to make "interface NAME" mandatory in switch definitions
which collapses our scenarios.
What are y'all's thoughts?
+--+
Carlos
>>>
>>>> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
>>>> index ef42549d105..0190c049837 100644
>>>> --- usr.sbin/vmd/priv.c
>>>> +++ usr.sbin/vmd/priv.c
>>>> @@ -88,6 +88,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
>>>> struct imsg *imsg)
>>>>switch (imsg->hdr.type) {
>>>>case IMSG_VMDOP_PRIV_IFDESCR:
>>>>case IMSG_VMDOP_PRIV_IFCREATE:
>>>> + case IMSG_VMDOP_PRIV_IFDESTROY:
>>>>case IMSG_VMDOP_PRIV_IFRDOMAIN:
>>>>case IMSG_VMDOP_PRIV_IFADD:
>>>>case IMSG_VMDOP_PRIV_IFUP:
>>>> @@ -125,6 +126,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
>>>> struct imsg *imsg)
>>>>errno != EEXIST)
>>>>log_warn("SIOCIFCREATE");
>>>>break;
>>>> + case IMSG_VMDOP_PRIV_IFDESTROY:
>>>> + /* Destroy the bridge */
>>>> + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
>>>> + if (ioctl(env->vmd_fd, SIOCIFDESTROY, ) < 0 &&
>>>> + errno != EEXIST)
>>>> + log_warn("SIOCIFDESTROY");
>>>> + break;
>>>>case IMSG_VMDOP_PRIV_IFRDOMAIN:
>>>>strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
>>>>ifr.ifr_rdomainid = vfr.vfr_id;
>>>> @@ -444,6 +452,23 @@ vm_priv_brconfig(struct privsep *ps, struct
>>>> vmd_switch *vsw)
>>>>return (0);
>>>> }
>>>>
>>>> +int
>>>> +vm_priv_brdestroy(struct privsep *ps, struct vmd_switch *vsw)
>>>> +{
>>>> + struct vmop_ifreqvfr;
>>>> +
>>>> + memset(, 0, sizeof(vfr));
>>>> +
>>>> + if (strlcpy(vfr.vfr_name, vsw->sw_ifname,
>>>> + sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
>>>> + return (-1);
>>>> +
>>>> + proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESTROY,
>>>> + , sizeof(vfr));
>>>> +
>>
[PATCH] tests for vmd config parsing
This patch adds a set of tests for vmd config parsing.
Comments? Ok?
diff --git regress/usr.sbin/Makefile regress/usr.sbin/Makefile
index 3912e794d4d..f19a656d45e 100644
--- regress/usr.sbin/Makefile
+++ regress/usr.sbin/Makefile
@@ -12,6 +12,7 @@ SUBDIR += relayd
SUBDIR += snmpd
SUBDIR += switchd
SUBDIR += syslogd
+SUBDIR += vmd
.if defined(REGRESS_FULL) || make(clean) || make(cleandir) || make(obj)
SUBDIR += pkg_add
diff --git regress/usr.sbin/vmd/Makefile regress/usr.sbin/vmd/Makefile
new file mode 100644
index 000..6c6671ada3f
--- /dev/null
+++ regress/usr.sbin/vmd/Makefile
@@ -0,0 +1,5 @@
+# $OpenBSD$
+
+SUBDIR += config
+
+.include
diff --git regress/usr.sbin/vmd/config/Makefile
regress/usr.sbin/vmd/config/Makefile
new file mode 100644
index 000..f5f58658af6
--- /dev/null
+++ regress/usr.sbin/vmd/config/Makefile
@@ -0,0 +1,32 @@
+# $OpenBSD$
+
+VMD ?= /usr/sbin/vmd
+
+VMD_PASS=boot-keyword memory-round memory-just-enough
+VMD_FAIL=kernel-keyword too-few-ram vm-name-too-long too-many-ifs \
+boot-name-too-long disk-path-too-long too-many-disks
+
+REGRESS_TARGETS=
+
+.for n in ${VMD_PASS}
+REGRESS_TARGETS += vmd-pass-${n}
+
+vmd-pass-${n}:
+ @echo ' $@ '
+ ${VMD} -n -f ${.CURDIR}/vmd-pass-${n}.conf 2>&1 | \
+ diff -u ${.CURDIR}/vmd-pass-${n}.ok /dev/stdin
+.endfor
+
+.for n in ${VMD_FAIL}
+REGRESS_TARGETS += vmd-fail-${n}
+
+vmd-fail-${n}:
+ @echo ' $@ '
+ ${VMD} -n -f ${.CURDIR}/vmd-fail-${n}.conf 2>&1 | \
+ cut -d : -f 2,3,4 | \
+ diff -u ${.CURDIR}/vmd-fail-${n}.ok /dev/stdin
+.endfor
+
+.PHONY: ${REGRESS_TARGETS}
+
+.include
diff --git regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.conf
regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.conf
new file mode 100644
index 000..bc569a9119e
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail on boot path (> 128)
+ramdisk="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/bsd.rd"
+vm "x" {
+boot $ramdisk
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.ok
regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.ok
new file mode 100644
index 000..56cb73b98cf
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-boot-name-too-long.ok
@@ -0,0 +1 @@
+5: kernel name too long
diff --git regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.conf
regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.conf
new file mode 100644
index 000..b70c3acf507
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.conf
@@ -0,0 +1,6 @@
+# $OpenBSD$
+# Fail on disk path (> 128)
+rdisk="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/bsd.img"
+vm "x" {
+disk $rdisk
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.ok
regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.ok
new file mode 100644
index 000..a384c812362
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-disk-path-too-long.ok
@@ -0,0 +1,2 @@
+disk path too long
+5: failed to parse disks:
/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/bsd.img
diff --git regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
new file mode 100644
index 000..3505def581b
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.conf
@@ -0,0 +1,12 @@
+# $OpenBSD$
+# Fail on kernel keyword; has been replaced by boot.
+ramdisk="/bsd.rd"
+switch "sw" {
+add vether0
+}
+vm "x" {
+kernel $ramdisk
+memory 1G
+disable
+interface { switch "sw" }
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
new file mode 100644
index 000..348817b1477
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-kernel-keyword.ok
@@ -0,0 +1 @@
+8: syntax error
diff --git regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.conf
regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.conf
new file mode 100644
index 000..f8b27056dea
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.conf
@@ -0,0 +1,5 @@
+# $OpenBSD$
+# Fail on memory (less than 1MB)
+vm "x" {
+memory 1048575
+}
diff --git regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok
regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok
new file mode 100644
index 000..0cf48a97eaf
--- /dev/null
+++ regress/usr.sbin/vmd/config/vmd-fail-too-few-ram.ok
@@ -0,0 +1,2 @@
+size must be at least one megabyte
+4: failed to parse size: 1048575
diff --git
[PATCH] VMD: Remove switch on reload prior to re-creating
Destroy switch on `vmctl reload` to allow SIOCBRDGADD to succeed
when creating new bridge and attaching interfaces to it.
Comments? Ok?
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index ef42549d105..0190c049837 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -88,6 +88,7 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct
imsg *imsg)
switch (imsg->hdr.type) {
case IMSG_VMDOP_PRIV_IFDESCR:
case IMSG_VMDOP_PRIV_IFCREATE:
+ case IMSG_VMDOP_PRIV_IFDESTROY:
case IMSG_VMDOP_PRIV_IFRDOMAIN:
case IMSG_VMDOP_PRIV_IFADD:
case IMSG_VMDOP_PRIV_IFUP:
@@ -125,6 +126,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
struct imsg *imsg)
errno != EEXIST)
log_warn("SIOCIFCREATE");
break;
+ case IMSG_VMDOP_PRIV_IFDESTROY:
+ /* Destroy the bridge */
+ strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
+ if (ioctl(env->vmd_fd, SIOCIFDESTROY, ) < 0 &&
+ errno != EEXIST)
+ log_warn("SIOCIFDESTROY");
+ break;
case IMSG_VMDOP_PRIV_IFRDOMAIN:
strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
ifr.ifr_rdomainid = vfr.vfr_id;
@@ -444,6 +452,23 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch
*vsw)
return (0);
}
+int
+vm_priv_brdestroy(struct privsep *ps, struct vmd_switch *vsw)
+{
+ struct vmop_ifreqvfr;
+
+ memset(, 0, sizeof(vfr));
+
+ if (strlcpy(vfr.vfr_name, vsw->sw_ifname,
+ sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
+ return (-1);
+
+ proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESTROY,
+ , sizeof(vfr));
+
+ return (0);
+}
+
uint32_t
vm_priv_addr(struct address *h, uint32_t vmid, int idx, int isvm)
{
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index f1abc54d9a3..834654a76de 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -852,7 +852,7 @@ int
vmd_reload(unsigned int reset, const char *filename)
{
struct vmd_vm *vm, *next_vm;
- struct vmd_switch *vsw;
+ struct vmd_switch *vsw, *next_vsw;
int reload = 0;
/* Switch back to the default config file */
@@ -885,6 +885,19 @@ vmd_reload(unsigned int reset, const char *filename)
}
}
+ TAILQ_FOREACH_SAFE(vsw, env->vmd_switches,
+ sw_entry, next_vsw) {
+ log_debug("%s: removing %s",
+ __func__, vsw->sw_ifname);
+ if (vm_priv_brdestroy(>vmd_ps,
+ vsw) == -1 ) {
+ log_warn("%s: failed to destroy switch",
+ __func__);
+ }
+ TAILQ_REMOVE(env->vmd_switches, vsw, sw_entry);
+ free(vsw);
+ }
+
/* Update shared global configuration in all children */
if (config_setconfig(env) == -1)
return (-1);
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 4b7b5f70495..fce5fcaaa60 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -100,6 +100,7 @@ enum imsg_type {
IMSG_VMDOP_PRIV_IFGROUP,
IMSG_VMDOP_PRIV_IFADDR,
IMSG_VMDOP_PRIV_IFRDOMAIN,
+ IMSG_VMDOP_PRIV_IFDESTROY,
IMSG_VMDOP_VM_SHUTDOWN,
IMSG_VMDOP_VM_REBOOT,
IMSG_VMDOP_CONFIG
@@ -323,6 +324,7 @@ int priv_findname(const char *, const char **);
int priv_validgroup(const char *);
int vm_priv_ifconfig(struct privsep *, struct vmd_vm *);
int vm_priv_brconfig(struct privsep *, struct vmd_switch *);
+int vm_priv_brdestroy(struct privsep *, struct vmd_switch *);
uint32_t vm_priv_addr(struct address *, uint32_t, int, int);
/* vmm.c */
--
2.14.2
[PATCH 0/2] VMD: handle VM termination error conditions better
This patch series handles two error conditions when terminating a VM (via vmctl or from within the VM). Comments? Ok? -- 2.14.2
[PATCH 1/2] VMD: Prevent Disappearing VM from vmctl status
Remove terminate_vm/vm_remove logic from vmm_dispatch_parent. This
logic is present in vmm_sighdlr when a VM process has signaled
SIGCHLD for proper cleanup.
diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
index ccd7680b479..8cc1c15157a 100644
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -170,15 +170,13 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p,
struct imsg *imsg)
else
res = 0;
} else {
- /* in the process of shutting down... */
- log_debug("%s: performing a forced shutdown",
- __func__);
+ /*
+* VM is currently being shutdown.
+* Check to see if the VM process is still
+* active. If not, return VMD_VM_STOP_INVALID.
+*/
vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
- /* ensure vm_id isn't 0 */
- if (vtp.vtp_vm_id != 0) {
- res = terminate_vm();
- vm_remove(vm);
- } else {
+ if (vtp.vtp_vm_id == 0) {
log_debug("%s: no vm running anymore",
__func__);
res = VMD_VM_STOP_INVALID;
--
2.14.2
[PATCH 2/2] VMD: Handle vm-induced powerdown better
The VMD parent process didn't handle the case of a VM exiting
with a non 0 return properly (i.e. EIO).
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index f1abc54d9a3..dcff6de0c0e 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -394,11 +394,14 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct
imsg *imsg)
case IMSG_VMDOP_TERMINATE_VM_EVENT:
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(vmr));
- log_debug("%s: handling TERMINATE_EVENT for vm id %d",
- __func__, vmr.vmr_id);
- if ((vm = vm_getbyvmid(vmr.vmr_id)) == NULL)
+ log_debug("%s: handling TERMINATE_EVENT for vm id %d ret %d",
+ __func__, vmr.vmr_id, vmr.vmr_result);
+ if ((vm = vm_getbyvmid(vmr.vmr_id)) == NULL) {
+ log_debug("%s: vm %d is no longer available",
+ __func__, vmr.vmr_id);
break;
- if (vmr.vmr_result == 0) {
+ }
+ if (vmr.vmr_result != EAGAIN) {
if (vm->vm_from_config) {
log_debug("%s: about to stop vm id %d",
__func__, vm->vm_vmid);
@@ -408,7 +411,7 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct
imsg *imsg)
__func__, vm->vm_vmid);
vm_remove(vm);
}
- } else if (vmr.vmr_result == EAGAIN) {
+ } else {
/* Stop VM instance but keep the tty open */
log_debug("%s: about to stop vm id %d with tty open",
__func__, vm->vm_vmid);
--
2.14.2
[PATCH v3 0/2] VMD: Prevent from crashing
This patch series contains: 1) Place plenty of log_debug statements in key places to assist with troubleshooting and debugging 2) Add new error code when attempting to stop a non-running vm (used by vmctl) Prevent vmd from crashing with updated logic when stopping a vm Difference between v2 and v3: v3 complies with style(9) v3 adds additional checks in vmm_dispatch_parent before calling terminate_vm in a force stop (i.e. calling `vmctl stop X` twice) Comments? Ok?
[PATCH v3 2/2] VMD: Prevent vmd crashing when stopping a stopped vm
* Fix logic handling stopping a VM. Prevents VMD from crashing.
* Add additional error code to notify the user that a vm cannot be
stopped when not running.
* Add additional log_debug statements.
diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
index 64d82ca847d..d1517d0d26d 100644
--- usr.sbin/vmctl/vmctl.c
+++ usr.sbin/vmctl/vmctl.c
@@ -206,7 +206,7 @@ vm_start_complete(struct imsg *imsg, int *ret, int
autoconnect)
break;
case VMD_DISK_INVALID:
warnx("specified disk image(s) are "
-"not regular files");
+ "not regular files");
*ret = ENOENT;
break;
default:
@@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
vmr = (struct vmop_result *)imsg->data;
res = vmr->vmr_result;
if (res) {
- errno = res;
- if (res == ENOENT)
+ switch (res) {
+ case VMD_VM_STOP_INVALID:
+ warnx("cannot stop vm that is not running");
+ *ret = EINVAL;
+ break;
+ case ENOENT:
warnx("vm not found");
- else
+ *ret = EIO;
+ break;
+ default:
warn("terminate vm command failed");
- *ret = EIO;
+ *ret = EIO;
+ }
} else {
warnx("sent request to terminate vm %d", vmr->vmr_id);
*ret = 0;
@@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
warnx("unexpected response received from vmd");
*ret = EINVAL;
}
+ errno = *ret;
return (1);
}
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 22da6d58a7b..1240339db52 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -54,6 +54,7 @@
#define VMD_BIOS_MISSING 1001
#define VMD_DISK_MISSING 1002
#define VMD_DISK_INVALID 1003
+#define VMD_VM_STOP_INVALID1004
/* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
#define VMD_DHCP_PREFIX"100.64.0.0/10"
diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
index 0e5ed1ed605..e3ff3be2f35 100644
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -150,29 +150,45 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p,
struct imsg *imsg)
if (id == 0) {
res = ENOENT;
- } else if ((vm = vm_getbyvmid(id)) != NULL &&
- vm->vm_shutdown == 0) {
- log_debug("%s: sending shutdown request to vm %d",
- __func__, id);
-
- /*
-* Request reboot but mark the VM as shutting down.
-* This way we can terminate the VM after the triple
-* fault instead of reboot and avoid being stuck in
-* the ACPI-less powerdown ("press any key to reboot")
-* of the VM.
-*/
- vm->vm_shutdown = 1;
- if (imsg_compose_event(>vm_iev,
- IMSG_VMDOP_VM_REBOOT, 0, 0, -1, NULL, 0) == -1)
- res = errno;
- else
- res = 0;
+ } else if ((vm = vm_getbyvmid(id)) != NULL) {
+ if (vm->vm_shutdown == 0) {
+ log_debug("%s: sending shutdown req to vm %d",
+ __func__, id);
+
+ /*
+* Request reboot but mark the VM as shutting
+* down. This way we can terminate the VM after
+* the triple fault instead of reboot and
+* avoid being stuck in the ACPI-less powerdown
+* ("press any key to reboot") of the VM.
+*/
+ vm->vm_shutdown = 1;
+ if (imsg_compose_event(>vm_iev,
+ IMSG_VMDOP_VM_REBOOT, 0, 0, -1, NULL, 0)
+ == -1)
+ res = errno;
+ else
+ res = 0;
+ } else {
+ /* in the process of shutting down... */
+
[PATCH v3 1/2] VMD: Place log_debug statements in key places
Add log_debug statements in key places to assist with
troubleshooting.
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 9ea87eb86e8..7c1eed2697a 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -81,14 +81,18 @@ config_purge(struct vmd *env, unsigned int reset)
struct vmd_switch *vsw;
unsigned int what;
+ log_debug("%s: purging vms and switches from running config",
+ __func__);
/* Reset global configuration (prefix was verified before) */
(void)host(VMD_DHCP_PREFIX, >vmd_cfg.cfg_localprefix);
/* Reset other configuration */
what = ps->ps_what[privsep_process] & reset;
if (what & CONFIG_VMS && env->vmd_vms != NULL) {
- while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL)
+ while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL) {
+ log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
+}
env->vmd_nvm = 0;
}
if (what & CONFIG_SWITCHES && env->vmd_switches != NULL) {
@@ -104,6 +108,7 @@ config_setconfig(struct vmd *env)
struct privsep *ps = >vmd_ps;
unsigned int id;
+ log_debug("%s: setting config", __func__);
for (id = 0; id < PROC_MAX; id++) {
if (id == privsep_process)
continue;
@@ -117,6 +122,7 @@ config_setconfig(struct vmd *env)
int
config_getconfig(struct vmd *env, struct imsg *imsg)
{
+log_debug("%s: retrieving config", __func__);
IMSG_SIZE_CHECK(imsg, >vmd_cfg);
memcpy(>vmd_cfg, imsg->data, sizeof(env->vmd_cfg));
@@ -129,6 +135,7 @@ config_setreset(struct vmd *env, unsigned int reset)
struct privsep *ps = >vmd_ps;
unsigned int id;
+ log_debug("%s: resetting state", __func__);
for (id = 0; id < PROC_MAX; id++) {
if ((reset & ps->ps_what[id]) == 0 ||
id == privsep_process)
@@ -147,6 +154,7 @@ config_getreset(struct vmd *env, struct imsg *imsg)
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(mode));
+ log_debug("%s: resetting state", __func__);
config_purge(env, mode);
return (0);
@@ -374,6 +382,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
free(tapfds);
}
+ log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
errno = saved_errno;
if (errno == 0)
@@ -406,6 +415,7 @@ config_getvm(struct privsep *ps, struct imsg *imsg)
imsg->fd = -1;
}
+ log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
if (errno == 0)
errno = EINVAL;
diff --git usr.sbin/vmd/control.c usr.sbin/vmd/control.c
index 1af8a0d5e14..5e103df08b9 100644
--- usr.sbin/vmd/control.c
+++ usr.sbin/vmd/control.c
@@ -276,6 +276,7 @@ control_close(int fd, struct control_sock *cs)
{
struct ctl_conn *c;
+ log_debug("%s", __func__);
if ((c = control_connbyfd(fd)) == NULL) {
log_warn("%s: fd %d: not found", __func__, fd);
return;
@@ -401,9 +402,14 @@ control_dispatch_imsg(int fd, short event, void *arg)
goto fail;
memcpy(, imsg.data, sizeof(vid));
vid.vid_uid = c->peercred.uid;
+ log_debug("%s vid: %d, name: %s, uid: %d",
+ __func__, vid.vid_id, vid.vid_name,
+ vid.vid_uid);
if (proc_compose_imsg(ps, PROC_PARENT, -1,
imsg.hdr.type, fd, -1, , sizeof(vid)) == -1) {
+ log_debug("%s: proc_compose_imsg failed...",
+ __func__);
control_close(fd, cs);
return;
}
diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c
index 183db93a678..6c5ff737471 100644
--- usr.sbin/vmd/proc.c
+++ usr.sbin/vmd/proc.c
@@ -756,6 +756,8 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid
id, int n,
proc_range(ps, id, , );
for (; n < m; n++) {
+ log_debug("%s: about to compose_event to PROC %d",
+ __func__, id);
if (imsg_compose_event(>ps_ievs[id][n],
type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
return (-1);
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index c7b9247d326..f561d1039ae 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -372,6 +372,8 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct
imsg *imsg)
case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(vmr));
+ log_debug("%s: forwarding TERMINATE VM
Re: [PATCH v2 2/2] VMD: Prevent vmd crashing when stopping a stopped vm
On 2017-09-05 23:55, Mike Larkin wrote:
> On Mon, Sep 04, 2017 at 12:03:31AM -0700, Carlos Cardenas wrote:
>> * Fix logic handling stopping a VM. Prevents VMD from crashing.
>> * Add additional error code to notify the user that a vm cannot be
>> stopped when not running.
>> * Add additional log_debug statements.
>>
>
> See below. Also this diff has tabs vs spaces problems like the
> previous one.
>
> If we fix the style issues and you can shed some light on the part
> I noted below, I think we can get both these diffs in.
>
> -ml
>
>> diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
>> index 64d82ca847d..5fb7fbfd74c 100644
>> --- usr.sbin/vmctl/vmctl.c
>> +++ usr.sbin/vmctl/vmctl.c
>> @@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>> vmr = (struct vmop_result *)imsg->data;
>> res = vmr->vmr_result;
>> if (res) {
>> -errno = res;
>> -if (res == ENOENT)
>> -warnx("vm not found");
>> -else
>> -warn("terminate vm command failed");
>> -*ret = EIO;
>> + switch (res) {
>> + case VMD_VM_STOP_INVALID:
>> + warnx("cannot stop vm that is not running");
>> + *ret = EINVAL;
>> + break;
>> + case ENOENT:
>> + warnx("vm not found");
>> + *ret = EIO;
>> + break;
>> + default:
>> + warn("terminate vm command failed");
>> + *ret = EIO;
>> + }
>> } else {
>> warnx("sent request to terminate vm %d", vmr->vmr_id);
>> *ret = 0;
>> @@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
>> warnx("unexpected response received from vmd");
>> *ret = EINVAL;
>> }
>> +errno = *ret;
>>
>> return (1);
>> }
>> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
>> index 22da6d58a7b..1240339db52 100644
>> --- usr.sbin/vmd/vmd.h
>> +++ usr.sbin/vmd/vmd.h
>> @@ -54,6 +54,7 @@
>> #define VMD_BIOS_MISSING1001
>> #define VMD_DISK_MISSING1002
>> #define VMD_DISK_INVALID1003
>> +#define VMD_VM_STOP_INVALID 1004
>>
>> /* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
>> #define VMD_DHCP_PREFIX "100.64.0.0/10"
>> diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
>> index d3233147cff..66083a5f959 100644
>> --- usr.sbin/vmd/vmm.c
>> +++ usr.sbin/vmd/vmm.c
>> @@ -169,10 +169,9 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p,
>> struct imsg *imsg)
>> else
>> res = 0;
>> } else {
>> -/* Terminate VMs that are unknown or shutting down */
>> -vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
>> -res = terminate_vm();
>> -vm_remove(vm);
>> +log_debug("%s: cannot stop vm that is not running",
>> +__func__);
>> +res = VMD_VM_STOP_INVALID;
>
> Is this really what we want? (Was this the part that was crashing vmd?)
> This will break (I think) stopping a vm, then while it is shutting down
> during vmmci shutdown processing, stopping it again to force kill it.
>
> Is the problem that we are doing vm_remove unconditionally, regardless of
> the result of the previous calls? (Eg, what if vm_vmid2id or terminate_vm
> failed?)
>
Using the previous patch in the series, this is what is observed when
attempting to stop a running vm (normal use case):
control_dispatch_imsg vid: 1, name: , uid: 0
proc_compose_imsg: about to compose_event to PROC 0
proc_compose_imsg: about to compose_event to PROC 2
vmm_dispatch_parent: recv'ed TERMINATE_VM for 1
vmm_dispatch_parent: sending shutdown request to vm 1
proc_compose_imsg: about to compose_event to PROC 0
vmd_dispatch_vmm: forwarding TERMINATE VM for 1
proc_compose_imsg: about to compose_event to PROC 1
control_close
vmm_sighdlr: handling signal
vmm_sighdlr: attempting to terminate vm 1
terminate_vm: term
[PATCH v2 0/2] VMD: Prevent from crashing
This patch series contains: 1) Place plenty of log_debug statements in key places to assist with troubleshooting and debugging 2) Add new error code when attempting to stop a non-running vm (used by vmctl) Prevent vmd from crashing with updated logic when stopping a vm Difference between v1 and v2: v2 leaves log.c alone since it's replicated/shared with many other daemons. All debug statements are now log_debug. Comments? Ok?
[PATCH v2 2/2] VMD: Prevent vmd crashing when stopping a stopped vm
* Fix logic handling stopping a VM. Prevents VMD from crashing.
* Add additional error code to notify the user that a vm cannot be
stopped when not running.
* Add additional log_debug statements.
diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
index 64d82ca847d..5fb7fbfd74c 100644
--- usr.sbin/vmctl/vmctl.c
+++ usr.sbin/vmctl/vmctl.c
@@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
vmr = (struct vmop_result *)imsg->data;
res = vmr->vmr_result;
if (res) {
- errno = res;
- if (res == ENOENT)
- warnx("vm not found");
- else
- warn("terminate vm command failed");
- *ret = EIO;
+ switch (res) {
+ case VMD_VM_STOP_INVALID:
+ warnx("cannot stop vm that is not running");
+ *ret = EINVAL;
+ break;
+ case ENOENT:
+ warnx("vm not found");
+ *ret = EIO;
+ break;
+ default:
+ warn("terminate vm command failed");
+ *ret = EIO;
+ }
} else {
warnx("sent request to terminate vm %d", vmr->vmr_id);
*ret = 0;
@@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
warnx("unexpected response received from vmd");
*ret = EINVAL;
}
+ errno = *ret;
return (1);
}
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 22da6d58a7b..1240339db52 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -54,6 +54,7 @@
#define VMD_BIOS_MISSING 1001
#define VMD_DISK_MISSING 1002
#define VMD_DISK_INVALID 1003
+#define VMD_VM_STOP_INVALID1004
/* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
#define VMD_DHCP_PREFIX"100.64.0.0/10"
diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
index d3233147cff..66083a5f959 100644
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -169,10 +169,9 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct
imsg *imsg)
else
res = 0;
} else {
- /* Terminate VMs that are unknown or shutting down */
- vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
- res = terminate_vm();
- vm_remove(vm);
+log_debug("%s: cannot stop vm that is not running",
+__func__);
+res = VMD_VM_STOP_INVALID;
}
cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE;
break;
@@ -346,6 +345,8 @@ vmm_sighdlr(int sig, short event, void *arg)
vmid = vm->vm_params.vmc_params.vcp_id;
vtp.vtp_vm_id = vmid;
+log_debug("%s: attempting to terminate vm %d",
+__func__, vm->vm_vmid);
if (terminate_vm() == 0) {
memset(, 0, sizeof(vmr));
vmr.vmr_result = ret;
@@ -505,7 +506,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
* supplied vm_terminate_params structure (vtp->vtp_vm_id)
*
* Parameters
- * vtp: vm_create_params struct containing the ID of the VM to terminate
+ * vtp: vm_terminate_params struct containing the ID of the VM to terminate
*
* Return values:
* 0: success
@@ -515,6 +516,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
int
terminate_vm(struct vm_terminate_params *vtp)
{
+log_debug("%s: terminating vmid %d", __func__, vtp->vtp_vm_id);
if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) < 0)
return (errno);
--
2.14.1
[PATCH v2 1/2] VMD: Place log_debug statements in key places
Add log_debug statements in key places to assist with
troubleshooting.
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 9ea87eb86e8..623a9b9d4f3 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -81,14 +81,18 @@ config_purge(struct vmd *env, unsigned int reset)
struct vmd_switch *vsw;
unsigned int what;
+log_debug("%s: purging vms and switches from running config",
+__func__);
/* Reset global configuration (prefix was verified before) */
(void)host(VMD_DHCP_PREFIX, >vmd_cfg.cfg_localprefix);
/* Reset other configuration */
what = ps->ps_what[privsep_process] & reset;
if (what & CONFIG_VMS && env->vmd_vms != NULL) {
- while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL)
+ while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL) {
+log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
+}
env->vmd_nvm = 0;
}
if (what & CONFIG_SWITCHES && env->vmd_switches != NULL) {
@@ -104,6 +108,7 @@ config_setconfig(struct vmd *env)
struct privsep *ps = >vmd_ps;
unsigned int id;
+log_debug("%s: setting config", __func__);
for (id = 0; id < PROC_MAX; id++) {
if (id == privsep_process)
continue;
@@ -117,6 +122,7 @@ config_setconfig(struct vmd *env)
int
config_getconfig(struct vmd *env, struct imsg *imsg)
{
+log_debug("%s: retrieving config", __func__);
IMSG_SIZE_CHECK(imsg, >vmd_cfg);
memcpy(>vmd_cfg, imsg->data, sizeof(env->vmd_cfg));
@@ -129,6 +135,7 @@ config_setreset(struct vmd *env, unsigned int reset)
struct privsep *ps = >vmd_ps;
unsigned int id;
+log_debug("%s: resetting state", __func__);
for (id = 0; id < PROC_MAX; id++) {
if ((reset & ps->ps_what[id]) == 0 ||
id == privsep_process)
@@ -147,6 +154,7 @@ config_getreset(struct vmd *env, struct imsg *imsg)
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(mode));
+log_debug("%s: resetting state", __func__);
config_purge(env, mode);
return (0);
@@ -374,6 +382,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
free(tapfds);
}
+log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
errno = saved_errno;
if (errno == 0)
@@ -406,6 +415,7 @@ config_getvm(struct privsep *ps, struct imsg *imsg)
imsg->fd = -1;
}
+log_debug("%s: calling vm_remove", __func__);
vm_remove(vm);
if (errno == 0)
errno = EINVAL;
diff --git usr.sbin/vmd/control.c usr.sbin/vmd/control.c
index 1af8a0d5e14..9263d0a3ae1 100644
--- usr.sbin/vmd/control.c
+++ usr.sbin/vmd/control.c
@@ -276,6 +276,7 @@ control_close(int fd, struct control_sock *cs)
{
struct ctl_conn *c;
+log_debug("%s", __func__);
if ((c = control_connbyfd(fd)) == NULL) {
log_warn("%s: fd %d: not found", __func__, fd);
return;
@@ -401,9 +402,14 @@ control_dispatch_imsg(int fd, short event, void *arg)
goto fail;
memcpy(, imsg.data, sizeof(vid));
vid.vid_uid = c->peercred.uid;
+log_debug("%s vid: %d, name: %s, uid: %d",
+__func__, vid.vid_id, vid.vid_name,
+vid.vid_uid);
if (proc_compose_imsg(ps, PROC_PARENT, -1,
imsg.hdr.type, fd, -1, , sizeof(vid)) == -1) {
+log_debug("%s: proc_compose_imsg failed...",
+__func__);
control_close(fd, cs);
return;
}
diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c
index 183db93a678..2bf0a98c8ee 100644
--- usr.sbin/vmd/proc.c
+++ usr.sbin/vmd/proc.c
@@ -756,6 +756,8 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid
id, int n,
proc_range(ps, id, , );
for (; n < m; n++) {
+log_debug("%s: about to compose_event to PROC %d", __func__,
+id);
if (imsg_compose_event(>ps_ievs[id][n],
type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
return (-1);
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index c7b9247d326..f34917e1a3b 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -372,6 +372,8 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct
imsg *imsg)
case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(vmr));
+
[PATCH 2/3] VMD: Place log_trace statements in key places
Add log_trace statements in key places to assist with
troubleshooting.
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 9ea87eb86e8..623a9b9d4f3 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -81,14 +81,18 @@ config_purge(struct vmd *env, unsigned int reset)
struct vmd_switch *vsw;
unsigned int what;
+log_trace("%s: purging vms and switches from running config",
+__func__);
/* Reset global configuration (prefix was verified before) */
(void)host(VMD_DHCP_PREFIX, >vmd_cfg.cfg_localprefix);
/* Reset other configuration */
what = ps->ps_what[privsep_process] & reset;
if (what & CONFIG_VMS && env->vmd_vms != NULL) {
- while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL)
+ while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL) {
+log_trace("%s: calling vm_remove", __func__);
vm_remove(vm);
+}
env->vmd_nvm = 0;
}
if (what & CONFIG_SWITCHES && env->vmd_switches != NULL) {
@@ -104,6 +108,7 @@ config_setconfig(struct vmd *env)
struct privsep *ps = >vmd_ps;
unsigned int id;
+log_trace("%s: setting config", __func__);
for (id = 0; id < PROC_MAX; id++) {
if (id == privsep_process)
continue;
@@ -117,6 +122,7 @@ config_setconfig(struct vmd *env)
int
config_getconfig(struct vmd *env, struct imsg *imsg)
{
+log_trace("%s: retrieving config", __func__);
IMSG_SIZE_CHECK(imsg, >vmd_cfg);
memcpy(>vmd_cfg, imsg->data, sizeof(env->vmd_cfg));
@@ -129,6 +135,7 @@ config_setreset(struct vmd *env, unsigned int reset)
struct privsep *ps = >vmd_ps;
unsigned int id;
+log_trace("%s: resetting state", __func__);
for (id = 0; id < PROC_MAX; id++) {
if ((reset & ps->ps_what[id]) == 0 ||
id == privsep_process)
@@ -147,6 +154,7 @@ config_getreset(struct vmd *env, struct imsg *imsg)
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(mode));
+log_trace("%s: resetting state", __func__);
config_purge(env, mode);
return (0);
@@ -374,6 +382,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
free(tapfds);
}
+log_trace("%s: calling vm_remove", __func__);
vm_remove(vm);
errno = saved_errno;
if (errno == 0)
@@ -406,6 +415,7 @@ config_getvm(struct privsep *ps, struct imsg *imsg)
imsg->fd = -1;
}
+log_trace("%s: calling vm_remove", __func__);
vm_remove(vm);
if (errno == 0)
errno = EINVAL;
diff --git usr.sbin/vmd/control.c usr.sbin/vmd/control.c
index 1af8a0d5e14..9263d0a3ae1 100644
--- usr.sbin/vmd/control.c
+++ usr.sbin/vmd/control.c
@@ -276,6 +276,7 @@ control_close(int fd, struct control_sock *cs)
{
struct ctl_conn *c;
+log_trace("%s", __func__);
if ((c = control_connbyfd(fd)) == NULL) {
log_warn("%s: fd %d: not found", __func__, fd);
return;
@@ -401,9 +402,14 @@ control_dispatch_imsg(int fd, short event, void *arg)
goto fail;
memcpy(, imsg.data, sizeof(vid));
vid.vid_uid = c->peercred.uid;
+log_trace("%s vid: %d, name: %s, uid: %d",
+__func__, vid.vid_id, vid.vid_name,
+vid.vid_uid);
if (proc_compose_imsg(ps, PROC_PARENT, -1,
imsg.hdr.type, fd, -1, , sizeof(vid)) == -1) {
+log_trace("%s: proc_compose_imsg failed...",
+__func__);
control_close(fd, cs);
return;
}
diff --git usr.sbin/vmd/proc.c usr.sbin/vmd/proc.c
index 183db93a678..2bf0a98c8ee 100644
--- usr.sbin/vmd/proc.c
+++ usr.sbin/vmd/proc.c
@@ -756,6 +756,8 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid
id, int n,
proc_range(ps, id, , );
for (; n < m; n++) {
+log_trace("%s: about to compose_event to PROC %d", __func__,
+id);
if (imsg_compose_event(>ps_ievs[id][n],
type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
return (-1);
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index c7b9247d326..f34917e1a3b 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -372,6 +372,8 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct
imsg *imsg)
case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
IMSG_SIZE_CHECK(imsg, );
memcpy(, imsg->data, sizeof(vmr));
+
[PATCH 0/3] VMD: Prevent from crashing
This patch series contains: 1) Add log_trace function (enabled when starting vmd with -vvv) 2) Place plenty of log_trace statements in key places to assist with troubleshooting and debugging 3) Add new error code when attempting to stop a non-running vm (used by vmctl) Prevent vmd from crashing with updated logic when stopping a vm Comments? Ok?
[PATCH 1/3] VMD: Add log_trace log target
Add log_trace log target for when verbose > 2
diff --git usr.sbin/vmd/log.c usr.sbin/vmd/log.c
index a6b0db9c264..55578f11e40 100644
--- usr.sbin/vmd/log.c
+++ usr.sbin/vmd/log.c
@@ -40,6 +40,8 @@ void log_info(const char *, ...)
__attribute__((__format__ (printf, 1, 2)));
void log_debug(const char *, ...)
__attribute__((__format__ (printf, 1, 2)));
+void log_trace(const char *, ...)
+ __attribute__((__format__ (printf, 1, 2)));
void logit(int, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
void vlog(int, const char *, va_list)
@@ -175,6 +177,18 @@ log_debug(const char *emsg, ...)
}
}
+void
+log_trace(const char *emsg, ...)
+{
+ va_list ap;
+
+ if (verbose > 2) {
+ va_start(ap, emsg);
+ vlog(LOG_DEBUG, emsg, ap);
+ va_end(ap);
+ }
+}
+
static void
vfatalc(int code, const char *emsg, va_list ap)
{
diff --git usr.sbin/vmd/proc.h usr.sbin/vmd/proc.h
index b91f3a5fecb..cce48d72137 100644
--- usr.sbin/vmd/proc.h
+++ usr.sbin/vmd/proc.h
@@ -208,6 +208,8 @@ voidlog_info(const char *, ...)
__attribute__((__format__ (printf, 1, 2)));
void log_debug(const char *, ...)
__attribute__((__format__ (printf, 1, 2)));
+void log_trace(const char *, ...)
+ __attribute__((__format__ (printf, 1, 2)));
void logit(int, const char *, ...)
__attribute__((__format__ (printf, 2, 3)));
void vlog(int, const char *, va_list)
--
2.14.1
[PATCH 3/3] VMD: Prevent vmd crashing when stopping a stopped vm
* Fix logic handling stopping a VM. Prevents VMD from crashing.
* Add additional error code to notify the user that a vm cannot be
stopped when not running.
* Add additional log_trace statements.
diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
index 64d82ca847d..5fb7fbfd74c 100644
--- usr.sbin/vmctl/vmctl.c
+++ usr.sbin/vmctl/vmctl.c
@@ -439,12 +439,19 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
vmr = (struct vmop_result *)imsg->data;
res = vmr->vmr_result;
if (res) {
- errno = res;
- if (res == ENOENT)
- warnx("vm not found");
- else
- warn("terminate vm command failed");
- *ret = EIO;
+ switch (res) {
+ case VMD_VM_STOP_INVALID:
+ warnx("cannot stop vm that is not running");
+ *ret = EINVAL;
+ break;
+ case ENOENT:
+ warnx("vm not found");
+ *ret = EIO;
+ break;
+ default:
+ warn("terminate vm command failed");
+ *ret = EIO;
+ }
} else {
warnx("sent request to terminate vm %d", vmr->vmr_id);
*ret = 0;
@@ -453,6 +460,7 @@ terminate_vm_complete(struct imsg *imsg, int *ret)
warnx("unexpected response received from vmd");
*ret = EINVAL;
}
+ errno = *ret;
return (1);
}
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 22da6d58a7b..1240339db52 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -54,6 +54,7 @@
#define VMD_BIOS_MISSING 1001
#define VMD_DISK_MISSING 1002
#define VMD_DISK_INVALID 1003
+#define VMD_VM_STOP_INVALID1004
/* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
#define VMD_DHCP_PREFIX"100.64.0.0/10"
diff --git usr.sbin/vmd/vmm.c usr.sbin/vmd/vmm.c
index d3233147cff..66083a5f959 100644
--- usr.sbin/vmd/vmm.c
+++ usr.sbin/vmd/vmm.c
@@ -169,10 +169,9 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct
imsg *imsg)
else
res = 0;
} else {
- /* Terminate VMs that are unknown or shutting down */
- vtp.vtp_vm_id = vm_vmid2id(vm->vm_vmid, vm);
- res = terminate_vm();
- vm_remove(vm);
+log_debug("%s: cannot stop vm that is not running",
+__func__);
+res = VMD_VM_STOP_INVALID;
}
cmd = IMSG_VMDOP_TERMINATE_VM_RESPONSE;
break;
@@ -346,6 +345,8 @@ vmm_sighdlr(int sig, short event, void *arg)
vmid = vm->vm_params.vmc_params.vcp_id;
vtp.vtp_vm_id = vmid;
+log_trace("%s: attempting to terminate vm %d",
+__func__, vm->vm_vmid);
if (terminate_vm() == 0) {
memset(, 0, sizeof(vmr));
vmr.vmr_result = ret;
@@ -505,7 +506,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
* supplied vm_terminate_params structure (vtp->vtp_vm_id)
*
* Parameters
- * vtp: vm_create_params struct containing the ID of the VM to terminate
+ * vtp: vm_terminate_params struct containing the ID of the VM to terminate
*
* Return values:
* 0: success
@@ -515,6 +516,7 @@ vmm_dispatch_vm(int fd, short event, void *arg)
int
terminate_vm(struct vm_terminate_params *vtp)
{
+log_trace("%s: terminating vmid %d", __func__, vtp->vtp_vm_id);
if (ioctl(env->vmd_fd, VMM_IOC_TERM, vtp) < 0)
return (errno);
--
2.14.1
[PATCH] VMD: Ensure disk is a regular file prior to vm boot
Add check(s) in vmd/vmctl to ensure a VM's disk are regular files.
Tested with the following:
vmctl start "test1" -d /dev/sd3c #block device
vmctl start "test2" -d /dev/rsd3c #char device
vmctl start "test3" -d fifo #named pipe
Comments? Ok?
diff --git usr.sbin/vmctl/vmctl.c usr.sbin/vmctl/vmctl.c
index f694f61e48c..e3db6a78c5b 100644
--- usr.sbin/vmctl/vmctl.c
+++ usr.sbin/vmctl/vmctl.c
@@ -204,6 +204,11 @@ vm_start_complete(struct imsg *imsg, int *ret, int
autoconnect)
warnx("could not find specified disk image(s)");
*ret = ENOENT;
break;
+ case VMD_DISK_INVALID:
+ warnx("specified disk image(s) are "
+"not regular files");
+ *ret = ENOENT;
+ break;
default:
errno = res;
warn("start vm command failed");
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 1e1166f8263..ced7ab666b4 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -157,6 +158,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
struct vmd_if *vif;
struct vmop_create_params *vmc = >vm_params;
struct vm_create_params *vcp = >vmc_params;
+ struct stat stat_buf;
unsigned int i;
int fd = -1, vmboot = 0;
int kernfd = -1, *diskfds = NULL, *tapfds = NULL;
@@ -225,6 +227,19 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
/* Open disk images for child */
for (i = 0 ; i < vcp->vcp_ndisks; i++) {
+/* Stat disk[i] to ensure it is a regular file */
+if (stat(vcp->vcp_disks[i], _buf) == -1) {
+ log_warn("%s: can't open disk %s", __func__,
+ vcp->vcp_disks[i]);
+ errno = VMD_DISK_MISSING;
+ goto fail;
+}
+if (S_ISREG(stat_buf.st_mode) == 0) {
+ log_warn("%s: disk %s is not a regular file", __func__,
+ vcp->vcp_disks[i]);
+ errno = VMD_DISK_INVALID;
+ goto fail;
+}
if ((diskfds[i] =
open(vcp->vcp_disks[i], O_RDWR)) == -1) {
log_warn("%s: can't open disk %s", __func__,
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index 57bdb71cd5f..daeffa7c80e 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -53,6 +53,7 @@
/* vmd -> vmctl error codes */
#define VMD_BIOS_MISSING 1001
#define VMD_DISK_MISSING 1002
+#define VMD_DISK_INVALID 1003
/* 100.64.0.0/10 from rfc6598 (IPv4 Prefix for Shared Address Space) */
#define VMD_DHCP_PREFIX"100.64.0.0/10"
--
2.14.1
[PATCH] vm.conf: Clarify VM name constraints
Add VM name constraints to match those in vmctl.8 manpage. Comments? Ok? diff --git usr.sbin/vmd/vm.conf.5 usr.sbin/vmd/vm.conf.5 index d1e68dbce5d..77a7a4e8cea 100644 --- usr.sbin/vmd/vm.conf.5 +++ usr.sbin/vmd/vm.conf.5 @@ -108,7 +108,9 @@ section starts with a declaration of the virtual machine .Ar name : .Bl -tag -width Ds .It Ic vm Ar name Brq ... -This name can be any string, and is typically a hostname. +The name can be any alphanumeric string along with '.', '-', and '_' +characters. However, it cannot start with '.', '-', or '_'. +Typically the name is a hostname. .El .Pp Followed by a block of parameters that is enclosed in curly brackets: -- 2.14.1
Re: [PATCH] add initial set of regress tests for vmd
On Sun, Aug 27, 2017 at 11:44 PM, Mike Larkin <mlar...@azathoth.net> wrote:
> On Sun, Aug 27, 2017 at 02:24:08PM -0700, Carlos Cardenas wrote:
> > Howdy.
> >
> > Below is patch for some initial parsing tests for vmd:
> > * Memory string parsing (too small, invalid size, and rounding)
> > * Max disk path
> > * Max vm name
> > * Max kernel path
> > * Max NICs
> >
> > More tests to come.
> >
> > One item missing is the proper hook into regress/usr.sbin/Makefile
> > as it was unknown if they should be run all the time or only for
> > REGRESS_FULL. Comments?
> >
> > Ok?
> >
>
> The diff doesn't apply. I think it got mangled. Can you send it again
> without line wrapping?
Re-sending.
+--+
Carlos
diff --git regress/usr.sbin/vmd/Makefile regress/usr.sbin/vmd/Makefile
new file mode 100644
index 000..d7b3794710a
--- /dev/null
+++ regress/usr.sbin/vmd/Makefile
@@ -0,0 +1,44 @@
+# $OpenBSD: $
+
+# TARGETS
+# vmd-pass: load vmd-pass-*.conf through vmd and check against
vmd-pass-*.ok
+# vmd-fail: load vmd-fail-*.conf through vmd and check against
vmd-fail-*.ok
+
+VMD_PASS=1 2 3
+VMD_FAIL=1 2 3 4 5 6 7
+
+MAKEOBJDIRPREFIX=
+
+SHELL=/bin/sh
+
+.MAIN: all
+
+.for n in ${VMD_PASS}
+VMD_PASS_TARGETS+=vmd_pass${n}
+
+vmd_pass${n}:
+ ${SUDO} vmd -n -f ${.CURDIR}/vmd-pass-${n}.conf 2>&1 | \
+ diff -u ${.CURDIR}/vmd-pass-${n}.ok /dev/stdin
+.endfor
+
+vmd-pass: ${VMD_PASS_TARGETS}
+REGRESS_TARGETS+=vmd-pass
+REGRESS_ROOT_TARGETS+=vmd-pass
+
+.for n in ${VMD_FAIL}
+VMD_FAIL_TARGETS+=vmd_fail${n}
+
+vmd_fail${n}:
+ ${SUDO} vmd -n -f ${.CURDIR}/vmd-fail-${n}.conf 2>&1 | \
+ diff -u ${.CURDIR}/vmd-fail-${n}.ok /dev/stdin
+.endfor
+
+vmd-fail: ${VMD_FAIL_TARGETS}
+REGRESS_TARGETS+=vmd-fail
+REGRESS_ROOT_TARGETS+=vmd-fail
+
+alltests: ${REGRESS_TARGETS}
+
+.PHONY: ${REGRESS_TARGETS}
+
+.include
diff --git regress/usr.sbin/vmd/vmd-fail-1.conf
regress/usr.sbin/vmd/vmd-fail-1.conf
new file mode 100644
index 000..3e3006a057d
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-1.conf
@@ -0,0 +1,12 @@
+# $OpenBSD: $
+# Fail on kernel keyword; has been replaced by boot.
+ramdisk="/bsd.rd"
+switch "sw" {
+add vether0
+}
+vm "x" {
+kernel $ramdisk
+memory 2G
+disable
+interface { switch "sw" }
+}
diff --git regress/usr.sbin/vmd/vmd-fail-1.ok
regress/usr.sbin/vmd/vmd-fail-1.ok
new file mode 100644
index 000..02fcd909b1e
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-1.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-1.conf:8: syntax error
diff --git regress/usr.sbin/vmd/vmd-fail-2.conf
regress/usr.sbin/vmd/vmd-fail-2.conf
new file mode 100644
index 000..69cf1c4e630
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-2.conf
@@ -0,0 +1,5 @@
+# $OpenBSD: $
+# Fail on memory (less than 1MB)
+vm "x" {
+memory 1048575
+}
diff --git regress/usr.sbin/vmd/vmd-fail-2.ok
regress/usr.sbin/vmd/vmd-fail-2.ok
new file mode 100644
index 000..4abf9f32b09
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-2.ok
@@ -0,0 +1,2 @@
+size must be at least one megabyte
+/usr/src/regress/usr.sbin/vmd/vmd-fail-2.conf:4: failed to parse size:
1048575
diff --git regress/usr.sbin/vmd/vmd-fail-3.conf
regress/usr.sbin/vmd/vmd-fail-3.conf
new file mode 100644
index 000..a8767829159
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-3.conf
@@ -0,0 +1,4 @@
+# $OpenBSD: $
+# Fail on VM name (> 32chars)
+vm "abcdefghijklmnopqrstuvwxyz0123456789" {
+}
diff --git regress/usr.sbin/vmd/vmd-fail-3.ok
regress/usr.sbin/vmd/vmd-fail-3.ok
new file mode 100644
index 000..788b7e53657
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-3.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-3.conf:3: vm name too long
diff --git regress/usr.sbin/vmd/vmd-fail-4.conf
regress/usr.sbin/vmd/vmd-fail-4.conf
new file mode 100644
index 000..83c59c2e61e
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-4.conf
@@ -0,0 +1,5 @@
+# $OpenBSD: $
+# Fail on nifs (> 4)
+vm "a" {
+nifs 5
+}
diff --git regress/usr.sbin/vmd/vmd-fail-4.ok
regress/usr.sbin/vmd/vmd-fail-4.ok
new file mode 100644
index 000..c533cba38d0
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-4.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-4.conf:4: syntax error
diff --git regress/usr.sbin/vmd/vmd-fail-5.conf
regress/usr.sbin/vmd/vmd-fail-5.conf
new file mode 100644
index 000..c53cfd32968
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-5.conf
@@ -0,0 +1,6 @@
+# $OpenBSD: $
+# Fail on boot path (> 128)
+ramdisk="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/bsd.rd"
+vm "x" {
+kernel $ramdisk
+}
diff --git regress/usr.sbin/vmd/vmd-fail-5.ok
regress/usr.sbin/vm
[PATCH] add initial set of regress tests for vmd
Howdy.
Below is patch for some initial parsing tests for vmd:
* Memory string parsing (too small, invalid size, and rounding)
* Max disk path
* Max vm name
* Max kernel path
* Max NICs
More tests to come.
One item missing is the proper hook into regress/usr.sbin/Makefile
as it was unknown if they should be run all the time or only for
REGRESS_FULL. Comments?
Ok?
+--+
Carlos
diff --git regress/usr.sbin/vmd/Makefile regress/usr.sbin/vmd/Makefile
new file mode 100644
index 000..d7b3794710a
--- /dev/null
+++ regress/usr.sbin/vmd/Makefile
@@ -0,0 +1,44 @@
+# $OpenBSD: $
+
+# TARGETS
+# vmd-pass: load vmd-pass-*.conf through vmd and check against
vmd-pass-*.ok
+# vmd-fail: load vmd-fail-*.conf through vmd and check against
vmd-fail-*.ok
+
+VMD_PASS=1 2 3
+VMD_FAIL=1 2 3 4 5 6 7
+
+MAKEOBJDIRPREFIX=
+
+SHELL=/bin/sh
+
+.MAIN: all
+
+.for n in ${VMD_PASS}
+VMD_PASS_TARGETS+=vmd_pass${n}
+
+vmd_pass${n}:
+ ${SUDO} vmd -n -f ${.CURDIR}/vmd-pass-${n}.conf 2>&1 | \
+ diff -u ${.CURDIR}/vmd-pass-${n}.ok /dev/stdin
+.endfor
+
+vmd-pass: ${VMD_PASS_TARGETS}
+REGRESS_TARGETS+=vmd-pass
+REGRESS_ROOT_TARGETS+=vmd-pass
+
+.for n in ${VMD_FAIL}
+VMD_FAIL_TARGETS+=vmd_fail${n}
+
+vmd_fail${n}:
+ ${SUDO} vmd -n -f ${.CURDIR}/vmd-fail-${n}.conf 2>&1 | \
+ diff -u ${.CURDIR}/vmd-fail-${n}.ok /dev/stdin
+.endfor
+
+vmd-fail: ${VMD_FAIL_TARGETS}
+REGRESS_TARGETS+=vmd-fail
+REGRESS_ROOT_TARGETS+=vmd-fail
+
+alltests: ${REGRESS_TARGETS}
+
+.PHONY: ${REGRESS_TARGETS}
+
+.include
diff --git regress/usr.sbin/vmd/vmd-fail-1.conf
regress/usr.sbin/vmd/vmd-fail-1.conf
new file mode 100644
index 000..3e3006a057d
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-1.conf
@@ -0,0 +1,12 @@
+# $OpenBSD: $
+# Fail on kernel keyword; has been replaced by boot.
+ramdisk="/bsd.rd"
+switch "sw" {
+add vether0
+}
+vm "x" {
+kernel $ramdisk
+memory 2G
+disable
+interface { switch "sw" }
+}
diff --git regress/usr.sbin/vmd/vmd-fail-1.ok
regress/usr.sbin/vmd/vmd-fail-1.ok
new file mode 100644
index 000..02fcd909b1e
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-1.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-1.conf:8: syntax error
diff --git regress/usr.sbin/vmd/vmd-fail-2.conf
regress/usr.sbin/vmd/vmd-fail-2.conf
new file mode 100644
index 000..69cf1c4e630
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-2.conf
@@ -0,0 +1,5 @@
+# $OpenBSD: $
+# Fail on memory (less than 1MB)
+vm "x" {
+memory 1048575
+}
diff --git regress/usr.sbin/vmd/vmd-fail-2.ok
regress/usr.sbin/vmd/vmd-fail-2.ok
new file mode 100644
index 000..4abf9f32b09
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-2.ok
@@ -0,0 +1,2 @@
+size must be at least one megabyte
+/usr/src/regress/usr.sbin/vmd/vmd-fail-2.conf:4: failed to parse size:
1048575
diff --git regress/usr.sbin/vmd/vmd-fail-3.conf
regress/usr.sbin/vmd/vmd-fail-3.conf
new file mode 100644
index 000..a8767829159
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-3.conf
@@ -0,0 +1,4 @@
+# $OpenBSD: $
+# Fail on VM name (> 32chars)
+vm "abcdefghijklmnopqrstuvwxyz0123456789" {
+}
diff --git regress/usr.sbin/vmd/vmd-fail-3.ok
regress/usr.sbin/vmd/vmd-fail-3.ok
new file mode 100644
index 000..788b7e53657
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-3.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-3.conf:3: vm name too long
diff --git regress/usr.sbin/vmd/vmd-fail-4.conf
regress/usr.sbin/vmd/vmd-fail-4.conf
new file mode 100644
index 000..83c59c2e61e
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-4.conf
@@ -0,0 +1,5 @@
+# $OpenBSD: $
+# Fail on nifs (> 4)
+vm "a" {
+nifs 5
+}
diff --git regress/usr.sbin/vmd/vmd-fail-4.ok
regress/usr.sbin/vmd/vmd-fail-4.ok
new file mode 100644
index 000..c533cba38d0
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-4.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-4.conf:4: syntax error
diff --git regress/usr.sbin/vmd/vmd-fail-5.conf
regress/usr.sbin/vmd/vmd-fail-5.conf
new file mode 100644
index 000..c53cfd32968
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-5.conf
@@ -0,0 +1,6 @@
+# $OpenBSD: $
+# Fail on boot path (> 128)
+ramdisk="/some/absolutepath/somewhere/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/abcdefghijklmnopqrstuvwxyz0123456789/bsd.rd"
+vm "x" {
+kernel $ramdisk
+}
diff --git regress/usr.sbin/vmd/vmd-fail-5.ok
regress/usr.sbin/vmd/vmd-fail-5.ok
new file mode 100644
index 000..f17303c6838
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-5.ok
@@ -0,0 +1 @@
+/usr/src/regress/usr.sbin/vmd/vmd-fail-5.conf:5: syntax error
diff --git regress/usr.sbin/vmd/vmd-fail-6.conf
regress/usr.sbin/vmd/vmd-fail-6.conf
new file mode 100644
index 000..5fc12df895f
--- /dev/null
+++ regress/usr.sbin/vmd/vmd-fail-6.conf
@@ -0,0 +1,6 @@
+# $OpenBSD: $
+# Fail on disk path (> 128)
[diff] typo in tls_load_file.3
Missing 'ocsp' in the function name. +--+ Carlos diff --git lib/libtls/man/tls_load_file.3 lib/libtls/man/tls_load_file.3 index fcaa5eef029..b83f55e0fe4 100644 --- lib/libtls/man/tls_load_file.3 +++ lib/libtls/man/tls_load_file.3 @@ -254,7 +254,7 @@ sets the files from which the public certificate, and private key will be read. .Fn tls_config_set_keypair_mem directly sets the public certificate, and private key from memory. .Pp -.Fn tls_config_set_keypair_file +.Fn tls_config_set_keypair_ocsp_file sets the files from which the public certificate, private key, and DER encoded OCSP staple will be read. .Pp
