Samba 3.6.0 offers a new spoolss architecture. Previously printer, driver and form information was stored in three separate tdbs (ntprinting.tdb, ntdrivers.tdb and ntforms.tdb). This information is now stored in Samba's registry. On spoolss initialisation nt_printing_tdb_migrate() is called to handle migration from previous formats, it does this by issuing a AddPrinterDriver request locally. In processing the AddPrinterDriver spoolss request, clean_up_driver_struct() is called which strips all directory components from driver file paths. move_driver_file_to_download_area() then assumes that files corresponding to the AddPrinterDriver request being handled are staged in the $architecture subdirectory of the print$ share path. This is where clients upload printer driver files prior to them being moved by the server into the $architecture\$version directory. As the driver files cannot be found (they are already in the $architecture\$version directory), the AddPrinterDriver request fails resulting in spoolss initialisation failure which in turn causes smbd to exit.
Created attachment 6550 [details] smbd log
The approach I'm looking at is to add a new entry to spoolss_AddPrinterDriverExFlags (or hijack APD_DONT_COPY_FILES_TO_CLUSTER) which causes the driver file IO to be skipped in AddPrinterDriver processing. Upgrading to blocker - failing to start smbd is disruptive.
Proposed fix, further testing required: The following changes since commit 314f161c00cfe3957f10b0f6f24adab737dfbe88: WHATSNEW: Update changes since rc1. (2011-06-07 20:13:47 +0200) are available in the git repository at: git://oss.sgi.com/ddiss/samba 36s-bso8214-spoolss-drv David Disseldorp (1): s3-printing: fix internal printing tdb upgrade librpc/idl/spoolss.idl | 3 ++- source3/printing/nt_printing_migrate.c | 11 ++++++----- source3/rpc_server/spoolss/srv_spoolss_nt.c | 22 +++++++++++++--------- 3 files changed, 21 insertions(+), 15 deletions(-) Feedback welcome.
Add the proposed fix as an attachment to this bug please ! Jeremy.
Created attachment 6554 [details] proposed driver upgrade fix
(In reply to comment #4) > Add the proposed fix as an attachment to this bug please ! > Jeremy. Sorry Jeremy, please see attached. The proposed change allows driver migration to successfully complete, however I'm now encountering problems in the printer migrate path. I'll raise a separate bug to track.
Re-assigning to Jeremy for patch review.
Patch sent to samba-technical for review along with bug 8235 change: http://lists.samba.org/archive/samba-technical/2011-June/078137.html
I'm sorry, I have to NAK this patch. We can't just grab a bit out of the spoolss_AddPrinterDriverExFlags bitmask and use it for our own purposes. Microsoft controls these values and can add their own flags at will. Can you fix this not to use a flag within this bitmask ? Is there somewhere else in this call we can flag this as internal ? Jeremy.
I'd like use to share the code between source3/utils/net_printing.c and source3/printing/nt_printing_migrate.c in future, as this is almost the same code. So I would like to have this code working against windows and samba. Wouldn't it make sense to upload the files (again) as a client would do? Or experiment with the other APD_ flags. Maybe we can use APD_COPY_FROM_DIRECTORY and specify the already existing path names. If that also against windows, we can add a short-cut in our code to skip them if the path is already the final one. Or we try to implement APD_STRICT_UPGRADE and upload empty dummy files with an older timestamp (If this also works against windows).
(In reply to comment #9) > Microsoft controls these values and can add their own flags at will. Can you > fix this not to use a flag within this bitmask ? Is there somewhere else in > this call we can flag this as internal ? Metze's suggestion of using APD_COPY_FROM_DIRECTORY sounds good. We could use that alongside APD_COPY_NEW_FILES and convert migrate_driver() to issue a spoolss_AddDriverInfo6 request (rather than info3). I'll see if I can get some test client code working against Windows and go from there.
Any update on this ? This is still set as blocker for 3.6.0. Jeremy.
(In reply to comment #12) > Any update on this ? This is still set as blocker for 3.6.0. Nothing yet, I'll work on this tomorrow.
Please note that the v3-6-test branch will be frozen on June 29. Thanks!
Created attachment 6639 [details] driver upgrade fix using APD_COPY_FROM_DIRECTORY APD_COPY_FROM_DIRECTORY has not yet been tested against a windows server so needs accompanying test code. For instance the fully qualified driver file paths specified with APD_COPY_FROM_DIRECTORY are interpreted by the Samba server as print$ specific paths which may be interpreted differently by Windows servers.
Re-assigning to Günther for patch review.
(In reply to comment #15) ... > APD_COPY_FROM_DIRECTORY has not yet been tested against a windows server so > needs accompanying test code. For instance the fully qualified driver file > paths specified with APD_COPY_FROM_DIRECTORY are interpreted by the Samba > server as print$ specific paths which may be interpreted differently by Windows > servers. As discussed with Günther yesterday, Windows (tested against Server 2k8) interprets paths carried in AddPrinterDriverEx requests with APD_COPY_FROM_DIRECTORY as UNC share paths.
Günther, please comment if this bug is still a blocker for the next 3.6 release. Thanks!
Günthers winreg based printing tdb migration changes are now in the tree, the same set of changes address this issue and bug 8235: e595590 s3-printing: remove tdb migration invalid printer name checks cfc3b6e s3-printing: make sure to first migrate the printers then the security descriptor. 5dd8185 s3-printing: fill info2_mask in printer migration 57bbb32 s3-printing: remove spoolss pipe from migration library, only using winreg finally. 8f3d5f5 s3-printing: use winreg interface for migration, instead of spoolss. 72b1f8b s3-printing: safe a ton of roundtrips by reusing existing winreg binding_handles. 0a1ec73 s3-printing: use winreg_internal functions. ada5380 s3-printing: add winreg_internal functions. a762eda s3-printing: add winreg_printer_binding_handle and remove most of srv_spoolss_util.c. f2be837 s3-printing: add rpc_client/cli_winreg_spoolss.c a0fc64a s3-waf: make LIBCLI_SPOOLSS a shared library 43cf3a2 s3-printing: move spoolss_create_default_devmode/secdesc to init_spoolss.h 74e4160 s3-printing: move driver_info_ctr_to_info8 to init_spoolss.h dd5375b s3-printing: move os2 related functions to printing/nt_printing_os2.c. 1caa7a8 s3-waf: Fix linking bugs causing segfaults. ff94539 s3-printing: open up a winreg pipe handle for the migration code. bafd721 s3-net: use printing_migrate library, and eliminate duplicate code. e02abd6 s3-printing: split out printing migration code into a smaller library. c9e3f6a s3-printing: skip migration of non-existent printers a36ce07 s3-printing: fill devicemode size in migrate_printer()
Created attachment 6677 [details] v3-6-test patch (backport from master)
Karolin, please add to 3.6.0
With this patchset, as David pointed out, this bug and bug #1194 are resolved
(In reply to comment #21) > Karolin, please add to 3.6.0 Pushed to v3-6-test. Closing out bug report. Thanks!
(In reply to comment #22) > With this patchset, as David pointed out, this bug and bug #1194 are resolved I think bug #8235 is meant, right?